-
Notifications
You must be signed in to change notification settings - Fork 501
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
commands: add debug as persistent flag #2660
Conversation
Allows using `--debug` to enable debug logging under any subcommand. Currently it needed to be set as `docker --debug buildx` meaning only way to enable debug in standalone mode was to set env variable instead and updating existing commands to add `--debug` was cumbersome. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
@@ -18,6 +18,7 @@ Build from a file | |||
| [`--builder`](#builder) | `string` | | Override the configured builder instance | | |||
| [`--call`](#call) | `string` | `build` | Set method for evaluating build (`check`, `outline`, `targets`) | | |||
| [`--check`](#check) | `bool` | | Shorthand for `--call=check` | | |||
| `-D`, `--debug` | `bool` | | Enable debug logging | |
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.
Do we want both the -D
and the --debug
to be inherited, or should we limit to only the long form (--debug
) for now? (can we?)
cc @jalonsogo
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 think it should be invisible to user if this flag is defined in the cli or buildx binary. docker --debug buildx
still uses the defined in CLI.
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.
Mostly looking here that the -D
is documented for the top-level, but for the subcommands it's a new flag, and in most (if not all) cases we refer to it as --debug
.
Where possibly I try to avoid shorthand flags if they're not critical to have (it's easy to add them later if there's a strong need), more so if they're case-sensitive (if there's both a -d
and -D
flag, which can be confusing).
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.
but for the subcommands it's a new flag, and in most (if not all) cases we refer to it as --debug.
--debug
is also a new flag for subcommand.
I don't think any user would understand why one flag formatting works in one place but not another.
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.
Could we have an integration test for checks with debug?
Allows using
--debug
to enable debug logging under any subcommand. Currently it needed to be set asdocker --debug buildx
meaning only way to enable debug in standalone mode was to set env variable instead and updating existing commands to add--debug
was cumbersome.