-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
refactor deprecation log event_api.tags.illegal #16545
refactor deprecation log event_api.tags.illegal #16545
Conversation
@mashhur I did a little refactor on deprecation message. Hope to get your view. Thank you. |
Quality Gate passedIssues Measures |
💚 Build Succeeded
|
@@ -267,7 +267,7 @@ class LogStash::Runner < Clamp::StrictCommand | |||
I18n.t("logstash.runner.flag.event_api.tags.illegal"), | |||
:default => LogStash::SETTINGS.get_default("event_api.tags.illegal"), | |||
:attribute_name => "event_api.tags.illegal", :passthrough => true, | |||
:deprecated_msg => I18n.t("logstash.runner.tags-illegal-deprecated") | |||
:obsoleted_version => "9" |
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.
The reason for the refactoring is, came with an idea to widely use obsoleted_version
with other deprecation params (such as http.*
). Change LGTM!
One thing I would like to clarify: merging against 8.x would make sense since event_api.tags.illegal
will be removed from main
(with this PR), how do you think?
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 need this PR to merge to main because the way of handling deprecation warning in clamp.rb
is benefit for the future deprecation not limit to 8.x
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!
@logstashmachine backport 8.x |
- add `obsoleted_version` and remove `deprecated_msg` from `deprecated_option` for consistent warning message (cherry picked from commit 8cd0fa8)
- add `obsoleted_version` and remove `deprecated_msg` from `deprecated_option` for consistent warning message (cherry picked from commit 8cd0fa8) Co-authored-by: kaisecheng <[email protected]>
Release notes
[rn:skip]
What does this PR do?
Use
obsoleted_version
to replacedeprecated_msg
to form a consistent warning messageWhy is it important/What is the impact to the user?
Make the warning consistent with other flags
Checklist
Author's Checklist
How to test this PR locally
Related issues
--event_api.tags.illegal
#16507