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

search: add two options #10095

Merged
merged 1 commit into from Dec 28, 2020
Merged

Conversation

hyuraku
Copy link
Contributor

@hyuraku hyuraku commented Dec 22, 2020

  • 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?
  • Have you successfully run brew man locally and committed any changes?

add two options to cmd/search for whether pull requests you search are open or closed`.


switch "--open",
depends_on: "--pull-request",
description: "Search for GitHub open pull requests"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: "Search for GitHub open pull requests"
description: "Search for only open GitHub pull requests"

description: "Search for GitHub open pull requests"
switch "--closed",
depends_on: "--pull-request",
description: "Search for GitHub closed pull requests"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
description: "Search for GitHub closed pull requests"
description: "Search for only closed GitHub pull requests"

Comment on lines 100 to 101
only = "open" if args.open? && !args.closed?
only = "closed" if args.closed? && !args.open?
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to make this a single if

def print_pull_requests_matching(query)
open_or_closed_prs = search_issues(query, type: "pr", user: "Homebrew")
def print_pull_requests_matching(query, only)
open_or_closed_prs = search_issues(query, is: only, type: "pr", user: "Homebrew")
Copy link
Member

Choose a reason for hiding this comment

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

Is this handled as expected when only is nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the result when only is nil is below.

brew search ruby --pul-request
==> Open pull requests
licensee 9.14.1 (new formula) (https://github.com/Homebrew/homebrew-core/pull/67513)
colorls 1.4.3 (new formula) (https://github.com/Homebrew/homebrew-core/pull/67165)
tpp: fix build with Xcode 12 (https://github.com/Homebrew/homebrew-core/pull/66517)
vowpal-wabbit 8.9.0 (https://github.com/Homebrew/homebrew-core/pull/64736)
dashing: update (https://github.com/Homebrew/homebrew-core/pull/65648)
install.sh: preload homebrew/core before brew update (https://github.com/Homebrew/install/pull/371)
tomee-plume 8.0.5 (https://github.com/Homebrew/homebrew-core/pull/65750)
Adds support for ManyCam (https://github.com/Homebrew/homebrew-cask/pull/91231)
minidlna 1.3.0 (https://github.com/Homebrew/homebrew-core/pull/65760)
Fix/m4/patch (https://github.com/Homebrew/linuxbrew-core/pull/21848)
mermaid-cli 8.8.4 (new formula) (https://github.com/Homebrew/homebrew-core/pull/65521)
openjdk: Add support for Apple silicon (https://github.com/Homebrew/homebrew-core/pull/65670)
alluxio 2.4.1 (https://github.com/Homebrew/homebrew-core/pull/65422)
autoconf 2.70 (https://github.com/Homebrew/homebrew-core/pull/66511)
geant4 10.7.0 (https://github.com/Homebrew/homebrew-core/pull/66422)
workflows: add remove-disabled-formulae workflow (https://github.com/Homebrew/homebrew-core/pull/67386)
Check flavour to prevent mixed architectures (https://github.com/Homebrew/brew/pull/9126)
go update gobootstrap on arm mac to a native arm version (https://github.com/Homebrew/homebrew-core/pull/67154)
onedrive-linux-client 2.4.5 (new formula) (https://github.com/Homebrew/homebrew-core/pull/64256)
ldc: use llvm (https://github.com/Homebrew/homebrew-core/pull/61117)
build(deps): bump rubocop from 1.5.1 to 1.6.1 in /Library/Homebrew (https://github.com/Homebrew/brew/pull/10050)

==> Closed pull requests
ruby-build 20201221 (https://github.com/Homebrew/homebrew-core/pull/67338)
utils/ruby: silence `which` errors when finding ruby (https://github.com/Homebrew/brew/pull/10034)
ruby-install 0.8.1 (https://github.com/Homebrew/homebrew-core/pull/67413)
utils/ruby.sh: simplify and fix Ruby-related logic (https://github.com/Homebrew/brew/pull/9472)
utils/ruby.sh: various tweaks (https://github.com/Homebrew/brew/pull/9544)
ruby-build 20201210 (https://github.com/Homebrew/homebrew-core/pull/66624)
ruby@2.4: deprecate! (https://github.com/Homebrew/homebrew-core/pull/66473)
ruby-build 20201208 (https://github.com/Homebrew/homebrew-core/pull/66470)
ruby-install 0.8.0 (https://github.com/Homebrew/homebrew-core/pull/66784)
cleanup: fix portable Ruby behaviour. (https://github.com/Homebrew/brew/pull/9479)
Fix macOS Ruby version handling (https://github.com/Homebrew/brew/pull/9465)
libprelude: explicitly disable Python 2 bindings and depend on ruby@2.6 for ruby bindings (https://github.com/Homebrew/linuxbrew-core/pull/21756)
ruby-build 20201118 (https://github.com/Homebrew/homebrew-core/pull/65111)
ruby-build 20201117 (https://github.com/Homebrew/homebrew-core/pull/65026)
Dependabot/bundler/library/homebrew/ruby macho 2.5.0 (https://github.com/Homebrew/brew/pull/10104)
tests.yml: force vendored Ruby on Catalina (https://github.com/Homebrew/homebrew-core/pull/66290)
Merge 2020-12-09 4f5a1e9aa59 (https://github.com/Homebrew/linuxbrew-core/pull/21752)
brew.sh: don't allow system Ruby on Catalina. (https://github.com/Homebrew/brew/pull/9442)
Improved no_usable_ruby() test (https://github.com/Homebrew/install/pull/338)
fq 0.13.4 (https://github.com/Homebrew/homebrew-core/pull/66917)
...

@@ -342,8 +342,8 @@ def merge_pull_request(repo, number:, sha:, merge_method:, commit_message: nil)
open_api(url, data: data, request_method: :PUT, scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES)
end

def print_pull_requests_matching(query)
open_or_closed_prs = search_issues(query, type: "pr", user: "Homebrew")
def print_pull_requests_matching(query, only)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def print_pull_requests_matching(query, only)
def print_pull_requests_matching(query, only: nil)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

def print_pull_requests_matching(query, only: nil) caused error like below.

$brew search ruby --pul-request
Error: wrong number of arguments (given 2, expected 1)
Please report this issue:
  https://docs.brew.sh/Troubleshooting
/usr/local/Homebrew/Library/Homebrew/utils/github.rb:345:in `print_pull_requests_matching'
/usr/local/Homebrew/Library/Homebrew/cmd/search.rb:106:in `search'
/usr/local/Homebrew/Library/Homebrew/brew.rb:124:in `<main>'

I will write def print_pull_requests_matching(query, only = nil)

Copy link
Member

Choose a reason for hiding this comment

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

def print_pull_requests_matching(query, only: nil) caused error like below.

Yes, all the callers will need updated too. I think my version is a bit more readable.

@MikeMcQuaid
Copy link
Member

@hyuraku Thanks again!

@MikeMcQuaid MikeMcQuaid merged commit 8795895 into Homebrew:master Dec 28, 2020
@hyuraku hyuraku deleted the add-options-to-search branch January 27, 2021 12:32
@BrewTestBot BrewTestBot added the outdated PR was locked due to age label Feb 27, 2021
@Homebrew Homebrew locked as resolved and limited conversation to collaborators Feb 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants