-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
server: Make --v2-deprecation=write-only the default and remove not-y… #13612
Conversation
d39d697
to
5794c68
Compare
@@ -17,7 +17,7 @@ package config | |||
type V2DeprecationEnum string | |||
|
|||
const ( | |||
// Default in v3.5. Issues a warning if v2store have meaningful content. | |||
// No longer supported in v3.6 | |||
V2_DEPR_0_NOT_YET = V2DeprecationEnum("not-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.
If it is not supported in 3.6, can we just remove it and remove the switch case at line 40?
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'm also usually for removing the dead code, however in this case I think it's worth to leave for historical context. Of course when V2 is fully removed this whole file should be deleted, however I think it's worth to keep this so that someone reading this file understands all previous enum stages.
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.
Sure, it isn't a big deal. Feel free to keep it unchanged.
It's OK because this file will eventually be removed.
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
…et option
Part of #12913