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

Give unstructured types their own DeepCopy methods #599

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

negz
Copy link
Member

@negz negz commented Nov 5, 2023

Description of your changes

Will fix crossplane/crossplane#4970 (after runtime bump)

These types all embed *unstructured.Unstructured. If they don't implement their own DeepCopy methods, callers will end up calling those of the embedded *Unstructured. The result is that a deepcopy isn't a real copy - it's the wrong type (*unstructured.Unstructured).

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

@negz negz requested review from a team as code owners November 5, 2023 22:39
@negz negz requested review from bobh66 and pedjak November 5, 2023 22:39
Fixes crossplane/crossplane#4970

These types all embed *unstructured.Unstructured. If they don't
implement their own DeepCopy methods, callers will end up calling those
of the embedded *Unstructured. The result is that a deepcopy isn't a
real copy - it's the wrong type (*unstructured.Unstructured).

Signed-off-by: Nic Cope <[email protected]>
Removes the tests - we don't need to test generated code.

Signed-off-by: Nic Cope <[email protected]>
@negz negz requested a review from phisco November 7, 2023 00:44
@negz negz merged commit 24db9cb into crossplane:master Nov 13, 2023
8 checks passed
@negz negz deleted the deeper-copy branch November 13, 2023 19:46
Copy link

Successfully created backport PR for release-1.14:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No-op claim -> composed applies are not skipped
4 participants