-
Notifications
You must be signed in to change notification settings - Fork 15.1k
Remove references for ingress-nginx on en,es,pt-br #53479
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
Remove references for ingress-nginx on en,es,pt-br #53479
Conversation
|
/cc @stormqueen1990 @edsoncelio @aojea |
|
/lgtm |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
+1 It's strange that the docs have ever had one random annotation added. |
|
/lgtm |
|
Typically we only do one language at a time, but I think for a cleanup PR like this it should be OK to get them in one go. /lgtm |
| When a IngressClass resource has this annotation set to `"true"`, new Ingress resource | ||
| without a class specified will be assigned this default class. | ||
|
|
||
| ### nginx.ingress.kubernetes.io/configuration-snippet |
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.
Why we added it: someone must have wanted to refer to it in a blog article I'd spotted that ingress-nginx was very sparsely documented, saw that it was using official annotations, and proposed that we register them.
I don't think we should remove it. It's OK to change, but we shouldn't try to rewrite history to the extent of pretending that the Kubernetes project never registered this annotation; we did.
There are open questions because in the last decade we use many more keys than when the project started tracking the set of known keys (as in key/value).
This is a registry. One purpose of a registry is to remember things indefinitely, to avoid duplicate use.
If there is a reason for an exception of our usual way of working, where we remember all keys previously recorded, let's be clear that we know the reason and agree on 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.
https://github.com/kubernetes/website/pull/53479/files#r2573068581 is a concern about switching away from a firmly established convention, where keys (as in key/value) stay registered forever, even if using them has become deprecated.
The IANA takes a similar approach and very rarely frees registered values to the extent of forgetting that they were ever registered. I think remembering the registration remains worthwhile.
So:
/lgtm cancel
Please read that comment and, if asking for an exception, clearly state and justify why it should be made.
(Changing the overall policy beyond an individual exception is also possible, but may well merit a KEP).
My advice: deprecate the annotation and delete the explanation of how you, a reader, might once have used it. We should do that even though there are other labels and annotations and taints that the project has not yet found capacity / time to document.
Precedents, especially precedents made for convenience, can be costly.
|
BTW, my take on how labels / annotations / taints get added to our registry:
|
@lmktfy Tim, I am confused why the above explanation by Ricardo doesn't supercede a rule/policy or is enough of a justification, especially given your comment: #53479 (comment) @kubernetes/sig-network-misc I would prefer SIG Network leads weigh in on the annotation here, rather than us being bogged down by process: we want our docs to first and foremost be helpful to users, especially if we're keeping an annotation that shouldn't be used. "Precedents, especially precedents made for convenience, can be costly." your mention here of a precedence for convenience doesn't feel right here. |
|
This issue was a mistake, and adding this annotation to the website should have never happened without someone from sig-network and ingress-nginx That said, @lmktfy your behavior with other contributors can be considered at least aggressive. People took their time to review the PR, people agreed with the PR and still because you don't agree with it you on a very individual decision goes and cancels others lgtm. I would like to ask for sig-docs leads to validate if your behavior is the one expected from others contributing to the documentation, and also kindly ask you to understand that the current situation requires a better general understanding (including we stopping to announce an unsupported annotation that should have never being part of the website). If you have any concerns or question, before canceling others lgtm, please reach to the sig chairs, or to the reviewers instead of enforcing your believes and understanding over others. Kubernetes is not a single person project and should never be. @kubernetes/sig-network-leads please be aware, this annotation should have never be part of the website, not removing it implies on it being constantly indexed by AI, search engines and also misleading users about it being officially supported |
|
I should have clarified my intent with the cancel of the LGTM. This repo is one of a very few with continuous deployment. So, approval is kind of risky: if someone looks at a PR, reads the short description and sees LGTM, they may reasonably conclude it's good to approve. So, because there is an open question about the way we manage the registry of annotations, I removed the LGTM. I would be delighted to have a way to flag a concern that wouldn't seem to as almost, uh, brutal, eg So, what I missed earlier was capturing that LGTM is "cheap" to re-add, and that people can. I do caution people not to add it without a good answer to the concerns I raised. Sorry for not writing a longer message on this earlier. In hindsight, I could have explained more about the context and why I picked those particular Prow commands. My ask to SIG Network is: respect that SIG Docs has conventions and policies. We can, and perhaps will make an exception to these, but - as an emeritus tech lead - I think people added LGTM without even knowing of the "we never delete registered annotations" guideline. Calling out the chance for an unintended drive by merge is part of the job. Doing it compassionately is too, and that's where I think I slipped up. I get that you, @rikatz, are saying you wouldn't have approved the earlier annotation registration PR that you now want to undo, but I stand by the points I wanted to make. This does need some kind of exception, and it's the need for that to be both discussed and considered that prompted me to put a pause on proceedings. We, SIG Docs, can help communicate project-eide changes. I am sure the SIGs involved will have plenty to learn and may want to plan in a retrospective. |
|
Just saw #53479 (comment) /lgtm |
|
LGTM label has been added. Git tree hash: d138068dff6cc8a9841c59c2e450de4665efc531
|
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: lmktfy 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 |
Can you share a link to these please? |
Description
This PR removes references for ingress-nginx on the following areas: