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.

Sign up for GitHub

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

Jump to bottom

📖 update and supplement webhook documentation with example to work with core types #4061

Merged
merged 1 commit into from
Sep 3, 2024
Merged

📖 update and supplement webhook documentation with example to work with core types #4061

merged 1 commit into from
Sep 3, 2024

Conversation

camilamacedo86
Copy link
Member

@camilamacedo86 camilamacedo86 commented Aug 9, 2024 •
edited
Loading

Closes: #3399
also, closes: #1241

@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2024
@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Aug 9, 2024
@camilamacedo86 camilamacedo86 changed the title WIP: update and supplement webhook docs 📖 update and supplement webhook documentation with example to work with core types Aug 9, 2024
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 9, 2024
@camilamacedo86 camilamacedo86 mentioned this pull request Aug 22, 2024
Closed
@camilamacedo86 camilamacedo86 force-pushed the update-doc-ref-webhooks branch from 53fbfca to 63eee41 Compare August 23, 2024 05:39
@camilamacedo86 camilamacedo86 requested a review from mythi August 23, 2024 05:40
mythi
mythi approved these changes Aug 23, 2024
View reviewed changes
Copy link

@mythi mythi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks good and is very informative, thanks

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: camilamacedo86, mythi

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

@camilamacedo86 camilamacedo86 force-pushed the update-doc-ref-webhooks branch 3 times, most recently from 1169d69 to 498643c Compare August 24, 2024 10:45
@camilamacedo86 camilamacedo86 changed the title 📖 update and supplement webhook documentation with example to work with core types (Blocked by #4060) 📖 update and supplement webhook documentation with example to work with core types Sep 2, 2024
@camilamacedo86
Copy link
Member Author

/hold

We need to get merged: #4060
So that we can have the convey and then have the examples here updated accordingly.

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 2, 2024
@camilamacedo86 camilamacedo86 force-pushed the update-doc-ref-webhooks branch from 498643c to b03ab90 Compare September 2, 2024 18:41
@camilamacedo86 camilamacedo86 changed the title (Blocked by #4060) 📖 update and supplement webhook documentation with example to work with core types 📖 update and supplement webhook documentation with example to work with core types Sep 2, 2024
camilamacedo86
camilamacedo86 commented Sep 2, 2024
View reviewed changes
Copy link
Member Author

@camilamacedo86 camilamacedo86 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are for a good time trying to fix this doc :-)
It has a LGTM
And the examples with CustomInterfaces are respecting the comments and the changes addressed in #4060.

So, it shows good enough to fly 🚀 since it is better than before. Therefore, moving forward with this one.

However, further improvements are welcome always.
If you see that need changes please, feel free to raise an PR with your suggestions.

/hold cancel

@camilamacedo86 camilamacedo86 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 2, 2024
@camilamacedo86
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Sep 3, 2024
@mythi
Copy link

mythi commented Sep 3, 2024

So, it shows good enough to fly 🚀 since it is better than before. Therefore, moving forward with this one.

+1, thanks!

@camilamacedo86
Copy link
Member Author

/skip pull-kubebuilder-e2e-k8s-1-28-7

@camilamacedo86 camilamacedo86 merged commit d97a77b into kubernetes-sigs:master Sep 3, 2024
10 of 11 checks passed
@camilamacedo86 camilamacedo86 deleted the update-doc-ref-webhooks branch September 3, 2024 06:32
@camilamacedo86 camilamacedo86 mentioned this pull request Oct 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mythi mythi mythi approved these changes

@rashmigottipati rashmigottipati Awaiting requested review from rashmigottipati

@varshaprasad96 varshaprasad96 Awaiting requested review from varshaprasad96

@sbueringer sbueringer Awaiting requested review from sbueringer

@alvaroaleman alvaroaleman Awaiting requested review from alvaroaleman

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. 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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

document when to use the high-level Defaulter/Validator interface vs the lower-level handler interfaces
3 participants
@camilamacedo86 @k8s-ci-robot @mythi