-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
π (go/v4): Start to check ENABLE_WEBHOOKS envvar into main.go to fix issue when webhooks are used #3578
π (go/v4): Start to check ENABLE_WEBHOOKS envvar into main.go to fix issue when webhooks are used #3578
Conversation
Hi @lowang-bh. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. 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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution π₯
Following some nits
- The plugins go/v2 and go/v3 are deprecated so changes should not be performed on them.
- After change the scaffold you need to run
make generate
so that all samples under testdata and docs will be properly updated. Could you please to do it?
8484658
to
334e8a2
Compare
Thank you. PS.: I also updated title for we have a better info when we generate the release notes. I hope that you do not mind. /ok-to-test |
79da654
to
57551df
Compare
Done.
Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/approved
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, lowang-bh 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 |
You need to run make generate =, see : https://github.com/kubernetes-sigs/kubebuilder/actions/runs/6063945828/job/16452451257?pr=3578#step:6:47 /hold |
/hold cancel |
Yes, I am looking into this issue. It always insert two pices of same code. |
fix make generate errors Signed-off-by: lowang-bh <lhui_wang@163.com> remove duplicated code in cmd/main.go when gen docs Signed-off-by: lowang-bh <lhui_wang@163.com>
57551df
to
bae7ac8
Compare
/test pull-kubebuilder-e2e-k8s-1-27-1 |
/test pull-kubebuilder-e2e-k8s-1-25-3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
The API fails because the change in the doc see: sigs.k8s.io/kubebuilder/v3/hack/docs/internal/cronjob-tutorial
Therefore, it is not a genuine breakchange. |
Fixes #3559
describe: fix issue that webhook generated code in cmd/main.go ignored checking the env ENABLE_WEBHOOKS
Test result :