-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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 os.Args hack for backwards compatible arguments #42209
remove os.Args hack for backwards compatible arguments #42209
Conversation
This pull request does not have a backport label.
To fixup this pull request, you need to add the backport labels for the needed
|
|
Pinging @elastic/elastic-agent-data-plane (Team:Elastic-Agent-Data-Plane) |
034a558
to
ecd404f
Compare
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 tested the beats locally as well as searching for all possible flags I could find in the help commands and subcommands of filebeat and metricbeat, couldn't find any leftover flags still using the single dash. LGTM.
@leehinman I have a question about the fallout of this change. I understand that this is a breaking change but there are numerous Elastic repositories, documentation, and scripts that use the single dash in the commands. How should we address those that are not part of the Beats repository? Is there a plan for handling this? |
- remove unused func - remove unused var
afd3023
to
62d1a2c
Compare
@andrewkroh @kruskall @zmoog any chance you can take a look at this PR? |
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
@vigneshshanmugam tagging as there are changed in heartbeat. We might want to check if there's impact on synthetic.
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 just not sure if that description needs to be fixed or not though
@@ -56,7 +56,7 @@ func main() { | |||
log.SetFlags(0) | |||
|
|||
if len(*fileOutput) > 0 { | |||
log.Fatalf("-output is configured, please use -folder flag instead to get the expected formatting of assets") | |||
log.Fatalf("-output is configured, please use --folder flag instead to get the expected formatting of assets") |
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.
log.Fatalf("-output is configured, please use --folder flag instead to get the expected formatting of assets") | |
log.Fatalf("--output is configured, please use --folder flag instead to get the expected formatting of assets") |
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.
committed. Thanks
Co-authored-by: Marc Guasch <[email protected]>
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
@aleksmaus any chance you could review this or recommend someone else from sec-deployment-and-devices? @lalit-satapathy any chance you review this or recommend someone else from obs-infraobs-integrations? Sorry to nag, but trying to get this breaking change in before 9.0. |
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.
Thank you!
@leehinman We have enough approvals to proceed, I'll force merge in order to give this enough time to soak into 9.0 release. |
*** Breaking Change ***
Proposed commit message
This PR removes support for single
-
multi-character command line options. You will need to use--
instead.For example
-environment
will no longer work, but--environment
will. The single charater options will continue to work, for example-d
and-c
.This greatly simplifies our command line argument parsing and makes it much easier to import the beats module into other projects.
Checklist
CHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Disruptive User Impact
If users have scripts that use the old single
-
arguments they will need to modify their scripts to use the double--
.Author's Checklist
How to test this PR locally
-environment container
fails but--environment container
succeedsRelated issues
Use cases
Screenshots
Logs