-
Notifications
You must be signed in to change notification settings - Fork 25.7k
Change setting's deprecation message wording #120718
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
Change setting's deprecation message wording #120718
Conversation
|
The only change to production code is: https://github.com/elastic/elasticsearch/pull/120718/files#diff-a533be0cdc0a71cb3c928730b6302742380726ce17f483fa82ae139ceeab78bc Everything else is tests |
|
It looks like this PR modifies one or more |
|
Warning It looks like this PR modifies one or more |
|
Pinging @elastic/es-core-infra (Team:Core/Infra) |
| private static final String DEPRECATED_MESSAGE_TEMPLATE = | ||
| "[{}] setting was deprecated in Elasticsearch and will be removed in a future release. " | ||
| + "See the %s changes documentation for the next major version."; | ||
| private static final String DEPRECATED_WARN_MESSAGE = String.format(Locale.ROOT, DEPRECATED_MESSAGE_TEMPLATE, "deprecation"); |
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 should use Strings.format (either class)
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.
Done
| private static final String DEPRECATED_MESSAGE_TEMPLATE = | ||
| "[{}] setting was deprecated in Elasticsearch and will be removed in a future release. " | ||
| + "See the %s changes documentation for the next major version."; | ||
| private static final String DEPRECATED_WARN_MESSAGE = String.format(Locale.ROOT, DEPRECATED_MESSAGE_TEMPLATE, "deprecation"); |
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.
'deprecation changes' doesn't make much sense, how about just 'See the deprecation documentation...' instead? Or 'deprecated functionality documentation'?
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.
Done: "deprecation changes documentation" -> "deprecation documentation", breaking changes message remains "breaking changes documentation"
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.
There's still some references to the old wording (and String.format(Locale.ROOT) in this PR eg in ESTestCase
💚 Backport successful
|
Depending on whether a message is critical or warning a message should indicate to check breaking changes documentation (critical level) or deprecation changes documentation (warn level) relates elastic#79666
Depending on whether a message is critical or warning a message should indicate to check breaking changes documentation (critical level) or deprecation changes documentation (warn level) relates elastic#79666
Depending on whether a message is critical or warning a message should
indicate to check breaking changes documentation (critical level) or
deprecation changes documentation (warn level)
relates #79666
Attempt to re-apply: #80900