-
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
📖 Fixed manager_webhook_patch not uncommented #3427
Conversation
Hi @lauchokyip. 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. |
docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml
Show resolved
Hide resolved
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.
/ok-to-test
@@ -36,7 +36,7 @@ patchesStrategicMerge: | |||
|
|||
# [WEBHOOK] To enable webhook, uncomment all the sections with [WEBHOOK] prefix including the one in | |||
# crd/kustomization.yaml | |||
#- manager_webhook_patch.yaml | |||
- manager_webhook_patch.yaml |
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.
Since you've already made the changes in the file hack/docs/internal/cronjob-tutorial/generate_cronjob.go
, do we really need the change here in this file? Asking because this will by overwritten by make generate
(and hence make generate-docs) as mentioned by @camilamacedo86 .
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.
The CI will run make generate
that's why this file is included as part of the changes.
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.
As I understand, make generate
will overwrite the file docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml
, right?
If so, then do we explicitly need to make this change here? Else, this can be skipped. 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.
make generate
will generate the file, that's why changes need to be done in hack/docs/internal/cronjob-tutorial/generate_cronjob.go
.
Once changes are made over there, I just run make generate
and docs/book/src/cronjob-tutorial/testdata/project/config/default/kustomization.yaml
was automatically been changed.
The CI will run make generate
and compare with the PR, and therefore it will fail if I didn't run make generate
before submitting the PR
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.
Please take a look at https://github.com/kubernetes-sigs/kubebuilder/actions/runs/5059859019/jobs/9085111581?pr=3427 for the Verify docs
session
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: camilamacedo86, lauchokyip, manavrajvanshi 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 |
@camilamacedo86 , I see the issue I reported has been closed since this PR has been merged. Just curious to know how long does it take for the changes to reflect on the Kubebuilder book? |
Fixes #3426