Skip to content

Conversation

@xrstf
Copy link
Contributor

@xrstf xrstf commented Apr 23, 2025

Summary

I am not sure why, but the various GroupName constants and +groupName comments were widely inconsistent in the examples. That lead to a working codegen, but when trying to actually use the generated fake client, one would end with errors like

W0422 13:58:15.416218 201389 reflector.go:569] pkg/mod/k8s.io/[email protected]/tools/cache/reflector.go:251: failed to list *v1.TestType: no kind "TestTypeList" is registered for version "example/v1" in scheme "pkg/runtime/scheme.go:100"

Note the API group example, which is neither example.dev nor example.some.corp and so things won't work.

It is also seemingly necessary to attach a +groupName to the doc.go of every version, not just their parent package.

I included the three sample tests I adopted from upstream. They helped me ensure it's all working and I guess can help us ensure it keeps working, so I added testing the examples module to make test.

Release Notes

Fix inconsistent API groups in the example APIs

@kcp-ci-bot kcp-ci-bot added dco-signoff: yes Indicates the PR's author has signed the DCO. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 23, 2025
@mjudeikis
Copy link
Contributor

/lgtm
/approve
/Hold
@gman0 can this merge? Will it impact a rebase?

@kcp-ci-bot kcp-ci-bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2025
@kcp-ci-bot kcp-ci-bot added the lgtm Indicates that a PR is ready to be merged. label Apr 24, 2025
@kcp-ci-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mjudeikis

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

The pull request process is described here

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

@kcp-ci-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: 78b6b39ddaa4f307ef7c0aa378366b3d12bc23f0

@kcp-ci-bot kcp-ci-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 24, 2025
@gman0
Copy link
Contributor

gman0 commented Apr 24, 2025

/lgtm

@gman0
Copy link
Contributor

gman0 commented Apr 24, 2025

All good @mjudeikis , thanks @xrstf !

@mjudeikis
Copy link
Contributor

/hold cancel

@kcp-ci-bot kcp-ci-bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 24, 2025
@kcp-ci-bot kcp-ci-bot merged commit f80885a into main Apr 24, 2025
8 of 9 checks passed
@kcp-ci-bot kcp-ci-bot deleted the fix-groupnames branch April 24, 2025 07:00
@xrstf
Copy link
Contributor Author

xrstf commented Apr 24, 2025

/cherrypick kcp-1.32.3

@kcp-ci-bot
Copy link
Contributor

@xrstf: new pull request created: #103

In response to this:

/cherrypick kcp-1.32.3

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.

@xrstf
Copy link
Contributor Author

xrstf commented Apr 24, 2025

/cherrypick kcp-1.31.6

@kcp-ci-bot
Copy link
Contributor

@xrstf: new pull request created: #104

In response to this:

/cherrypick kcp-1.31.6

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

approved Indicates a PR has been approved by an approver from all required OWNERS files. dco-signoff: yes Indicates the PR's author has signed the DCO. lgtm Indicates that a PR is ready to be merged. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants