-
Notifications
You must be signed in to change notification settings - Fork 14.8k
Fix gate status for ComponentSLIs in 1.32 #49340
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
base: main
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
✅ Pull request preview available for checking
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Would it also make sense to update https://github.com/kubernetes/website/blob/main/content/zh-cn/docs/reference/command-line-tools-reference/feature-gates/ComponentSLIs.md?plain=1 at the same time?
PRs should only update a single localization (we might one day be able to make other localizations use the English localization as a source of truth for feature state - that'd be handy if we could) |
@@ -13,6 +13,12 @@ stages: | |||
- stage: beta | |||
defaultValue: true | |||
fromVersion: "1.27" | |||
toVersion: "1.31" | |||
- stage: stable | |||
locked: true |
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.
This is OK, but maybe slightly misleading; we don't support a locked
key in anything I know of.
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.
locked
is gonna be important, because a feature which has lockedToDefault: true
will not be shown from the help message such as kube-apiserver --help
.
The gate is there, not removed. You cannot change its setting. A weird state it is.
We will need to revise the shortcode someday to reflect this.
Adding locked: true
is a first step. At least, it is doing no harm at the moment.
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.
How about: file a frozen issue about that improvement around locked
. Then we've something to refer back to, and if we close the issue we'll know we ought to do cleanup.
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.
Filed one: #49388
@@ -13,6 +13,12 @@ stages: | |||
- stage: beta | |||
defaultValue: true | |||
fromVersion: "1.27" | |||
toVersion: "1.31" | |||
- stage: stable | |||
locked: true |
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.
locked: true |
as #49388 isn't implemented yet.
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.
Does it hurt?
Do you mean we revisit this features again to find if they are locked or not later?
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.
I think there is a cost here; people might see this field and infer it's implemented. Finding out that it's not can be a surprise, replicated across multiple contributors.
I can see a small chance of a painful surprise, where maybe someone relies on this value and they shouldn't have.
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.
It is not about you think. People read the rendered HTML, not the YAML.
I'm all for removing this information if you sign up to add this back.
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.
Noted; I still have the qualm I had before.
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.
@tengqm would you be willing to start a Slack discussion about the way forward?
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
No description provided.