ARGV: stop supporting --homebrew-developer option #48322
Conversation
Homebrew developers have the corresponding variable permanently set in their environment and wanting to appear like a Homebrew developer for a single invocation is exceedingly rare. Additionally, the option won't be recognized by `bin/brew`. (It is also undocumented.) Using `HOMEBREW_DEVELOPER=1 brew <command>` is still possible, and not more inconvenient than passing the `--homebrew-developer` option.
| @@ -125,7 +125,7 @@ def git? | |||
| end | |||
|
|
|||
| def homebrew_developer? | |||
| include?("--homebrew-developer") || !ENV["HOMEBREW_DEVELOPER"].nil? | |||
| !ENV["HOMEBREW_DEVELOPER"].nil? | |||
There was a problem hiding this comment.
Isn’t it strange to have a method defined on ARGV that now have nothing to do with ARGV? 😉
There was a problem hiding this comment.
I agree but it feels weird to not have the API kept.
There was a problem hiding this comment.
We could maybe add this on ENV and keep this method for compatibility; then at some point in the future remove it when we’ll be used to the new API?
There was a problem hiding this comment.
I didn't even realize how misplaced this now looks. On the bright side, this keeps the required changes to a minimum. I'm a bit torn on how to best resolve this, as moving this to ENV doesn't feel appropriate (the extensions there are mostly concerned with the build process).
Maybe something like Config could be created for things that can be passed both via arguments and the environment? The environment variables are effectively configuration, we just don't seem to like an approach where we would put them into a file, something a lot of software allows.
There was a problem hiding this comment.
Yeah we talked about it a bit on the chat; it would be great to move the ARGV extension methods in a separate module for all these global settings. The long-term goal is to remove as many references to ARGV as possible outside of cmd/ to have more predictable/testable code (i.e. calling Homebrew.foo shouldn’t depend on the arguments on the command-line; if it needs them we should call e.g. Homebrew.foo(:verbose => true)) and a clear separation of the core vs. the code that handles what is given on the command-line, so we’ll be able to e.g. automatically generate manpage/completion scripts with all the options, warn if an option is not recognized, etc.
|
👍 |
|
No objections. |
|
@BrewTestBot test this please. |
|
Sorry if I'm bothering you all once more, but is that fine to merge as-is? From the discussion I'm not entirely sure if this is the case or if we'd like to postpone this until we have a better way of handling “configuration”, i.e. stuff that responds to environment variables and sometimes to arguments, too. |
|
Merge as-is |
Homebrew developers have the corresponding variable permanently set in their environment and wanting to appear like a Homebrew developer for a single invocation is exceedingly rare. Additionally, the option won't be recognized by `bin/brew`. (It is also undocumented.) Using `HOMEBREW_DEVELOPER=1 brew <command>` is still possible, and not more inconvenient than passing the `--homebrew-developer` option. Closes Homebrew#48322. Signed-off-by: Martin Afanasjew <martin@afanasjew.de>
Homebrew developers have the corresponding variable permanently set in their environment and wanting to appear like a Homebrew developer for a single invocation is exceedingly rare. Additionally, the option won't be recognized by
bin/brew. (It is also undocumented.)Using
HOMEBREW_DEVELOPER=1 brew <command>is still possible, and not more inconvenient than passing the--homebrew-developeroption.