Skip to content
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

Add ShellCommand module for shell command stubs #17758

Merged
merged 13 commits into from
Jul 15, 2024
Merged

Conversation

ZhongRuoyu
Copy link
Member

  • 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.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Follow-up to #17742, implementing @Bo98's idea #17742 (comment). A RuboCop has also been added to make sure each stub has its counterpart.

Library/Homebrew/shell_command.rb Outdated Show resolved Hide resolved
Library/Homebrew/cmd/vendor-install.rb Outdated Show resolved Hide resolved
Library/Homebrew/dev-cmd/rubocop.rb Outdated Show resolved Hide resolved
@ZhongRuoyu ZhongRuoyu force-pushed the shell-command-stubs branch from 038e3a1 to 8851752 Compare July 15, 2024 18:48
@ZhongRuoyu ZhongRuoyu force-pushed the shell-command-stubs branch from 8851752 to 5fc0fe2 Compare July 15, 2024 18:49
@ZhongRuoyu ZhongRuoyu enabled auto-merge July 15, 2024 19:46
@ZhongRuoyu ZhongRuoyu merged commit 628d34b into master Jul 15, 2024
24 checks passed
@ZhongRuoyu ZhongRuoyu deleted the shell-command-stubs branch July 15, 2024 19:53
Comment on lines 9 to +10
class Repository < AbstractCommand
include ShellCommand
Copy link
Member

Choose a reason for hiding this comment

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

@ZhongRuoyu What's the thoughts on why include ShellCommand was a better fit here than class Repository < ShellCommand?

Copy link
Member

Choose a reason for hiding this comment

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

Haven't checked but the command system might detect ShellCommand as a command itself if it's a subclass of AbstractCommand.

@MikeMcQuaid
Copy link
Member

Nice work here, thanks @ZhongRuoyu!

I think a nice follow-up would be to rip out all the #: -handling logic for manpages now that we're not using them any more.

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.

3 participants