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
ARGV: Replace ARGV.verbose? with Homebrew.args.verbose? #6983
Conversation
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.
nice! Can ARGV.verbose?
be deleted yet?
dc322e7
to
8e9b49e
Compare
8e9b49e
to
7dfc3f2
Compare
@@ -127,7 +127,7 @@ def exec(*args) | |||
end | |||
end | |||
|
|||
if @failed && ARGV.verbose? |
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.
@MikeMcQuaid Sandbox.exec
runs in a separate thread, replacing this with Homebrew.args.verbose?
does not work as CLI::Parser
is not instantiated yet - so global args dependent on environment vars will not be set.
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.
We could fix this either by
- Directly checking if
HOMEBREW_VERBOSE
is set instead ofHomebrew.args.verbose?
- Decoupling global args from
CLI::Parser
instantiation. - Any other approach??
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.
- Directly checking if
HOMEBREW_VERBOSE
is set instead ofHomebrew.args.verbose?
This seems reasonable for a one-time thing 👍
7dfc3f2
to
1fd5997
Compare
1fd5997
to
acde828
Compare
|
Thanks again @GauthamGoli ! |
brew style
with your changes locally?brew tests
with your changes locally?#5730