-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
📖added supported labels and annotations refrence document #7418
📖added supported labels and annotations refrence document #7418
Conversation
Hi @hackeramitkumar. 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.
/ok-to-test
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.
Thanks for taking this on! I've got a few changes that might help to make this easier to read and understand.
docs/book/src/SUMMARY.md
Outdated
@@ -108,3 +108,4 @@ | |||
- [Code Review in Cluster API](./REVIEWING.md) | |||
- [Version Support](./reference/versions.md) | |||
- [Roadmap](./roadmap.md) | |||
- [Supported Labels/Annotations](./reference/labels_and_annotations.md) |
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.
Could we also add a link to this in the API reference? i.e. ./reference/api_reference.md
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.
Did it.
|
||
|label |note | | ||
|:--------:|:--------:| | ||
| cluster.x-k8s.io/cluster-name| this label is set on machines linked to a cluster and external objects(bootstrap and infrastructure providers) | |
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.
| cluster.x-k8s.io/cluster-name| this label is set on machines linked to a cluster and external objects(bootstrap and infrastructure providers) | | |
| cluster.x-k8s.io/cluster-name| this label is set on machines linked to a cluster and external objects (bootstrap and infrastructure providers) | |
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.
Did it.
|
||
|annotation | note | | ||
|:--------:|:--------:| | ||
| topology.cluster.x-k8s.io/managed-field-paths | this annotation is used to store the list of paths managed by the topology controller. changes to those paths will be considered authoritative. | |
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.
| topology.cluster.x-k8s.io/managed-field-paths | this annotation is used to store the list of paths managed by the topology controller. changes to those paths will be considered authoritative. | | |
| topology.cluster.x-k8s.io/managed-field-paths | This annotation is used to store the list of paths managed by the topology controller. changes to those paths will be considered authoritative. | |
Could you capitalize the first letter of sentences throughout?
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.
did it.
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.
good to see labels being documented
089fc2b
to
97e7b68
Compare
0f2f306
to
28adb17
Compare
@alexander-demichev please review the PR. I have now made changes according to you and killianmuldoon. |
This is a great start, thank you!
Ideally, we should also define some consistent sort criteria for both tables |
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.
/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.
Minor nits, otherwise 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.
Depending on which one merges first, we should add #7107 to this doc
cc @jackfrancis
28adb17
to
1826ad1
Compare
@furkatgofurov7 , You have to add a "/" before 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.
/lgtm
Please open an issue to track follow up work as per the comment above
Yeah sure. |
1826ad1
to
cfefedb
Compare
@sbueringer , I have removed "topology.cluster.x-k8s.io/managed-field-paths" annotation and made all the columns to left justified. Please review it. |
@hackeramitkumar /cherry-pick release-1.2 |
@sbueringer: once the present PR merges, I will cherry-pick it on top of release-1.2 in a new PR and assign it to you. In response to this:
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. |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: sbueringer 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 |
@sbueringer: new pull request created: #7488 In response to this:
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. |
What this PR does / why we need it:
I have added all the supported labels and annotation in a single refrence document. I have added a .md file in refrence section , which contains two tables one for labels and another for annotations.
Fixes #5394