Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add shareProcessNamespace capability to v1alpha2 #2541

Merged

Conversation

frzifus
Copy link
Member

@frzifus frzifus commented Jan 18, 2024

Description:

Link to tracking Issue:

Closes #2540

Testing:

Documentation:

Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
@frzifus frzifus requested a review from a team January 18, 2024 17:18
@jaronoff97 jaronoff97 added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 18, 2024
Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to run make generate

@frzifus
Copy link
Member Author

frzifus commented Jan 18, 2024

afaik we do not generate v1alpha2, right?

@frzifus frzifus requested a review from jaronoff97 January 18, 2024 17:24
@swiatekm
Copy link
Contributor

afaik we do not generate v1alpha2, right?

We don't build manifests, but we do generate the deepcopy code.

@frzifus
Copy link
Member Author

frzifus commented Jan 18, 2024

@swiatekm-sumo Is coping a primitive values not covered in line 600? Since make generate does not produce any output. While when I change the field to be a ptr, I can see that the generator adds a few lines to copy the value. Once I change it back and run make generate it gets removed. 🐎

// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil.
func (in *OpenTelemetryCommonFields) DeepCopyInto(out *OpenTelemetryCommonFields) {
*out = *in

@swiatekm
Copy link
Contributor

I was confused why the documentation change was in this PR, but it looks like it's because the original PRs didn't include it. Which is strange, because the CI should fail that. I'll look into it, but this PR seems fine even though the changes are confusing.

@frzifus
Copy link
Member Author

frzifus commented Jan 18, 2024

Oh, I thought the generated lines belong to v1alpha2. But It seems that not part of api.md 🐰 🤦

@jaronoff97 jaronoff97 merged commit d6f3c31 into open-telemetry:main Jan 18, 2024
27 of 28 checks passed
@frzifus frzifus deleted the v1alphav2_share_process_namespace branch January 18, 2024 19:05
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
Signed-off-by: Benedikt Bongartz <bongartz@klimlive.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add shareProcessName to common fields for v1alpha2
4 participants