-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Switch binaries to use posix flags #7052
Conversation
69cc13e
to
abc222d
Compare
abc222d
to
20ce45f
Compare
"fmt" | ||
"os" | ||
|
||
"github.com/influxdata/influxdb/client" | ||
"github.com/influxdata/influxdb/cmd/influx/cli" | ||
flag "github.com/spf13/pflag" |
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.
nit: I would prefer we don't alias this. It's confusing as when you read the code later you think it's the standard library flag package.
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 agree. I'm not keen on aliasing to std library package names.
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 library is intended to be a drop-in replacement to the flag package and is supposed to be functionally identical. The user of the library isn't supposed to care if this is pflag
or flag
, which is why this is aliased. Because of the way this is meant to be used, it could also be easy to accidentally intermix two different packages without this alias.
Is this change primarily for future support? I might of missed it, but I'm not sure what commands benefitted by moving to posix style flags. |
It's mostly to get us to be more similar to other commands on posix systems. I also added a few short options that are listed within the help. Primarily, |
Updated help messages, man pages, and replaced the go flag library with github.com/spf13/pflag for posix compatible flags. Also fixed a bug with help pages where the command wasn't rewritten correctly. Now `influxd help run` works correctly again. Updated scripts to use the new flags and normalized tabs to spaces within the bash scripts.
20ce45f
to
d6aeed5
Compare
This is tagged revisit in the future so I'm going to close this and keep the branch around. |
Updated help messages, man pages, and replaced the go flag library with
github.com/spf13/pflag for posix compatible flags. Also fixed a bug with
help pages where the command wasn't rewritten correctly. Now
influxd help run
works correctly again.Updated scripts to use the new flags and normalized tabs to spaces
within the bash scripts.