Refactor commands to remove 'require "cmd/*"'

[apjanke]

• Have you followed the guidelines in our Contributing document?
• Have you checked to ensure there aren't other open Pull Requests for the same change?
• Have you added an explanation of what your changes do and why you'd like us to include them?
• Have you written new tests for your changes? Here's an example. No: no feature changes; only a refactor.
• Have you successfully run brew style with your changes locally?
• Have you successfully run brew tests with your changes locally?

---

This is an attempt to refactor install, upgrade, and help to remove their use of require "cmd/*" imports.

Fixes #4059.

Does this style of refactoring look good to you folks? If so, I'll do the rest, except for search, which is handled in #4253.

[apjanke]

BTW, the style failure is this:

$ brew style ./cmd/install.rb                                                                            ✘ 1 refactor-require-cmd ✭
Warning: unrecognized cop Rspec/ExpectActual found in .rubocop.yml
== cmd/install.rb ==
C:300: 23: Method parameter must be at least 3 characters long.

1 file inspected, 1 offense detected

Complaining about:

def install_formula(f)

Are we actually requiring longer argument names now? I seem to recall quite a few of these function(f) functions.

[MikeMcQuaid]

Does this style of refactoring look good to you folks? If so, I'll do the rest, except for search, which is handled in #4253.

Yes, definitely, thanks and great work here!

Are we actually requiring longer argument names now?
Are we actually requiring longer argument names now? I seem to recall quite a few of these function(f) functions.

I think ideally, yeh. Make it formula. Thanks!

[apjanke]

Cool.

I don't remember: do you accept PRs for style-only fixes? I could make a pass of style fixes after doing this refactor; there's a lot of files with style complaints right now.

[reitermarkus]

I don't remember: do you accept PRs for style-only fixes? I could make a pass of style fixes after doing this refactor; there's a lot of files with style complaints right now.

Not sure what you mean here, brew style should always pass before merging a PR.

[reitermarkus]

I think the problem is that brew style ./cmd/install.rb uses the Formula-specific style rules.

[apjanke]

I think the problem is that brew style ./cmd/install.rb uses the Formula-specific style rules.

Oh, yep, that's what's going on. When you pass a file argument, it pulls in the "audit" rules, regardless of whether that file is a formula or something else.

No problem; I can just run plain brew style from now on. It's fast enough.

[apjanke]

I've updated this PR to include other commands. Combined with #4253, this will cover all the commands that are importing "cmd/*". Then we need to refactor the tests, but I'll leave that for another PR.

[MikeMcQuaid]

Can you make both the above block ifs (and refactor the unless .. ||). I know not your code but nice to refactor this so it's not wider than the GitHub diff UI.

[MikeMcQuaid]

I've updated this PR to include other commands. Combined with #4253, this will cover all the commands that are importing "cmd/*". Then we need to refactor the tests, but I'll leave that for another PR.

Nice! One nit but otherwise 👍 👏.

[MikeMcQuaid]

Library/Homebrew/cmd/style.rb

Rebase needed for this.

[apjanke]

Amended to rebase and refactor that nit out. (Had to modify the nit code a bit more to satisfy brew style.)

[MikeMcQuaid]

Thanks again @apjanke!