-
Notifications
You must be signed in to change notification settings - Fork 889
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
Add separate subcommands for encryption, decryption, rotating, editing, and setting values #1343
base: main
Are you sure you want to change the base?
Conversation
e08d227
to
4b4f1c4
Compare
4b4f1c4
to
0a6d2e4
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 saw that you tried to remove duplicated flags. Here are a few remaining ones if you want.
cli.BoolFlag{ | ||
Name: "yes, y", | ||
Usage: `pre-approve all changes and run non-interactively`, | ||
}, |
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.
Still duplicated
[]cli.Flag{ | ||
cli.StringFlag{ | ||
Name: "file, f", | ||
Usage: "the file to add the group to", |
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.
Maybe we should write the file to delete the group key from
cli.BoolFlag{ | ||
Name: "yes, y", | ||
Usage: `pre-approve all changes and run non-interactively`, | ||
}, |
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.
Duplicated yes flag.
cli.BoolFlag{ | ||
Name: "verbose", | ||
Usage: "Enable verbose logging output", | ||
}, |
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.
Still duplicated
cli.BoolFlag{ | ||
Name: "verbose", | ||
Usage: "Enable verbose logging output", | ||
}, |
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.
Still duplicated
cli.BoolFlag{ | ||
Name: "background", | ||
Usage: "background the process and don't wait for it to complete", | ||
}, |
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.
Duplicated.
cli.BoolFlag{ | ||
Name: "background", | ||
Usage: "background the process and don't wait for it to complete", | ||
}, |
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.
Duplicated
7e50f4a
to
1e64a07
Compare
1e64a07
to
8b1c7ce
Compare
My goal with this PR for now is to check out how creating subcommands could work. There are quite a few PRs waiting to get processed that add a lot of rebase work to this one, so I might also just ditch it and re-start from scratch. Thanks for marking the other ones, I'll take a look at them at some point once this is more stable... |
When warning, also inform about potentially ignored flags. Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
Signed-off-by: Felix Fontein <felix@fontein.de>
8b1c7ce
to
fe65b43
Compare
@felixfontein can this PR be closed, now that #1391 is merged? |
This is mainly still open for the flag de-duplication. I wanted to continue working on this once some more PRs are merged, which unfortunately never happened until now. |
This is a Work in Progress for exploring how to implement #1333. Right now the new subcommands are basically a stripped-down copy'n'paste of the current "main" command.
Fixes #1333