-
Notifications
You must be signed in to change notification settings - Fork 48
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
Documentation for all controllers of etcd-druid #722
Documentation for all controllers of etcd-druid #722
Conversation
Skipping CI for Draft Pull Request. |
Thank you @renormalize for your contribution. Before I can start building your PR, a member of the organization must set the required label(s) {'reviewed/ok-to-test'}. Once started, you can check the build status in the PR checks section below. |
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.
@renormalize thanks for the well-written PR, and for documenting the design and functionality of the controllers in a concise way. I have left some suggestions for improvements. PTAL.
* Documents all controllers of etcd-druid in docs/concepts/00-controllers.md * Explains the usage of Kubebuilder * Explains the package structure * Introduces the *controller manager* * Briefly explains the 5 controllers
* Changed the file name to docs/concepts/controllers.md * Added all suggestions to the content for each controller
d6c928d
to
bdb9a37
Compare
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.
@renormalize thanks a lot for making all the requested changes! Just couple more nits, but overall LGTM 👍
* Removed unecessary comment. * Grammar fix in line 53.
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.
@renormalize thanks for addressing all my comments. LGTM!
/lgtm
/retest |
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
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.
Overall looks good.
just few nits.
/retest |
How to categorize this PR?
/area documentation
/kind enhancement
What this PR does / why we need it:
Documentation for the 5 controllers of etcd-druid
docs/concepts/controllers.md
Which issue(s) this PR fixes:
Fixes #695
Special notes for your reviewer:
Due to the planned changes to the
etcd
andcustodian
controllers, the documentation for those two is not complete at the moment.Release note: