Skip to content

fix: bind flags during PreRunE rather than init - #66

Merged
setrofim merged 1 commit into
mainfrom
setrofim/fix
Nov 10, 2026
Merged

setrofim merged 1 commit into
mainfrom
setrofim/fix

Conversation

@setrofim

@setrofim setrofim commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

We use one global viper instance for configuration (shared between all commands), however Flag objects exist per-command. When we were binding flags to the viper object, during command creation, they were overwriting each other, so that flags for only one of the verify commands ended up being bound to config, and the flags for other commands were being ignored (only verify commands use bindings, other commands do not and are unaffected by this).

Binding during PreRunE ensures that the binding happens before RunE (where the config is read) but only for the command that is actually being run.

We use one global viper instance for configuration (shared between all
commands), however Flag objects exist per-command. When we were binding
flags during command creation, they were overwriting each other, so that
flags for only one of the verify commands ended up being bound to
config, and the flags for other commands were being ignored (only
verify commands use bindings, other commands do not and are
unaffected by this).

Binding during PreRunE ensures that the binding happens before RunE
(where the config is read) but only for the command that is actually
being run.

Signed-off-by: Sergei Trofimov <sergei.trofimov@arm.com>
@setrofim
setrofim requested a review from a team as a code owner November 9, 2026 12:10
@setrofim

setrofim commented Nov 9, 2026

Copy link
Copy Markdown
Contributor Author

Cc @anta5010

@yogeshbdeshpande yogeshbdeshpande left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@setrofim
setrofim merged commit 2a4a33e into main Nov 10, 2026
9 checks passed
@setrofim
setrofim deleted the setrofim/fix branch November 10, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants