Port remaining commands to use AbstractCommand #16998
Conversation
dduugg
changed the title
dduugg
changed the title
| UNBREWED_EXCLUDE_FILES = %w[.DS_Store].freeze | ||
| UNBREWED_EXCLUDE_PATHS = %w[ |
There was a problem hiding this comment.
Would it be too paranoid to make these private constants?
| ($stdout.tty? || super) && !quiet? | ||
| end | ||
| def verbose? | ||
| ($stdout.tty? || Context.current.verbose?) && !Context.current.quiet? |
There was a problem hiding this comment.
The context mixin is another option here.
| next | ||
| end | ||
| Migrator.migrate_if_needed(formula, force: args.force?) | ||
| Homebrew.reinstall_formula( |
There was a problem hiding this comment.
This should probably get namespaced properly at some point. It seems to get used in both the upgrade and reinstall commands.
| require "cmd/shared_examples/args_parse" | ||
|
|
||
| RSpec.describe Homebrew::Cmd::RbenvSync do | ||
| it_behaves_like "parseable arguments" |
There was a problem hiding this comment.
I wonder if we could just automatically run it_behaves_like "parseable arguments" on all abstract command classes. The would remove the need to add this for every command and make it impossible to forget to add this test.
There was a problem hiding this comment.
I'll move the untap module back into this class after this gets merged in as discussed in #16814 (comment).
|
Thanks again @dduugg and to @apainintheneck for the review! |
This comment was marked as off-topic.
This comment was marked as off-topic.
Please do not complain about things breaking before running |
Thats a fair comment, that said, for some reason the install script (https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh), in certain environments, is installing an outdated version of brew - I will take a look into it. Also wasn't really complaining and I closed my own comment as off topic ¯_(ツ)_/¯, really appreciate the work y'all do was just jarring this morning (I have a GitHub Action that installs brew each time, using the install script, and for some reason it's installing 4.2.14, instead of the latest version, and this was a change that happened over night but I see no updates to the install script in a few weeks so IDK what's going on there and not blaming anyone anyway) |
brew stylewith your changes locally?brew typecheckwith your changes locally?brew testswith your changes locally?Continuation of #16975
Resolves #16814 (though there will be some cleanup PRs)