Skip to content

[release-0.11] 🐛 Change the type of markers.Collector.byPackage's key from string to *loader.Package#833

Closed
k8s-infra-cherrypick-robot wants to merge 2 commits intokubernetes-sigs:release-0.11from
k8s-infra-cherrypick-robot:cherry-pick-792-to-release-0.11
Closed

[release-0.11] 🐛 Change the type of markers.Collector.byPackage's key from string to *loader.Package#833
k8s-infra-cherrypick-robot wants to merge 2 commits intokubernetes-sigs:release-0.11from
k8s-infra-cherrypick-robot:cherry-pick-792-to-release-0.11

Conversation

@k8s-infra-cherrypick-robot
Copy link

@k8s-infra-cherrypick-robot k8s-infra-cherrypick-robot commented Jul 12, 2023

This is an automated cherry-pick of #792

Some more text for the PR verifier

/assign sbueringer

ntoofu added 2 commits July 12, 2023 18:03
Markers might be lost in generated CRD when the package containing
markers is referenced by several packages and CRD is generated by
those packages.

For more details, see kubernetes-sigs#783.
Markers are saved per `*ast.TypeSpec`, which is specific to `Package` object.
`*ast.TypeSpec` in another `Package` object is different even when
the `Package.ID` is the same, and so collected markers cannot be reused
for another `Package` object.

Therefore, `Package` itself is used as a key for cache of markers
insetead of `Package.ID`.

For more details about the bug, see kubernetes-sigs#783.
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: k8s-infra-cherrypick-robot
Once this PR has been reviewed and has the lgtm label, please assign pwittrock for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested a review from pwittrock July 12, 2023 18:03
@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 12, 2023
@sbueringer
Copy link
Member

sbueringer commented Jul 12, 2023

Oh don't need the fix in release-0.11 (although no objection to merge there).
Just missed that we have a v0.12 release because we don't have a release branch for v0.12 yet
#792 (comment)

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 12, 2023
@sbueringer
Copy link
Member

Let's close this. I only accidentally opened this PR for this branch

We can consider reopening/merging if someone asks for it

/close

@k8s-ci-robot
Copy link
Contributor

@sbueringer: Closed this PR.

Details

In response to this:

Let's close this. I only accidentally opened this PR for this branch

We can consider reopening/merging if someone asks for it

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

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

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants