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.

Sign up for GitHub

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

Jump to bottom

Add closed PR check to bump cmds #14396

Merged
merged 1 commit into from Jan 23, 2023
Merged

Add closed PR check to bump cmds #14396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 8 additions & 5 deletions Library/Homebrew/dev-cmd/bump-cask-pr.rb
View file
Open in desktop
Expand Up @@ -91,7 +91,9 @@ def bump_cask_pr
old_version = cask.version
old_hash = cask.sha256

check_open_pull_requests(cask, args: args)
check_pull_requests(cask, state: "open", args: args)
# if we haven't already found open requests, try for an exact match across closed requests
check_pull_requests(cask, state: "closed", args: args, version: new_version) if new_version.present?

old_contents = File.read(cask.sourcefile_path)

Expand Down Expand Up @@ -193,12 +195,13 @@ def bump_cask_pr
GitHub.create_bump_pr(pr_info, args: args)
end

def check_open_pull_requests(cask, args:)
def check_pull_requests(cask, state:, args:, version: nil)
tap_remote_repo = cask.tap.remote_repo || cask.tap.full_name
GitHub.check_for_duplicate_pull_requests(cask.token, tap_remote_repo,
state: "open",
file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
args: args)
state: state,
version: version,
file: cask.sourcefile_path.relative_path_from(cask.tap.path).to_s,
args: args)
end

def run_cask_audit(cask, old_contents, args:)
Expand Down
21 changes: 14 additions & 7 deletions Library/Homebrew/dev-cmd/bump.rb
View file
Open in desktop
Expand Up @@ -26,7 +26,7 @@ def bump_args
switch "--cask", "--casks",
description: "Check only casks."
switch "--open-pr",
description: "Open a pull request for the new version if there are none already open."
description: "Open a pull request for the new version if none have been opened yet."
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is my attempt at clarifying that matching open or closed PRs will block you from opening another one. It's still probably too subtle though.

flag "--limit=",
description: "Limit number of package results returned."
flag "--start-with=",
Expand Down Expand Up @@ -213,9 +213,9 @@ def livecheck_result(formula_or_cask)
"error: #{e}"
end

def retrieve_pull_requests(formula_or_cask, name)
def retrieve_pull_requests(formula_or_cask, name, state:, version: nil)
tap_remote_repo = formula_or_cask.tap&.remote_repo || formula_or_cask.tap&.full_name
pull_requests = GitHub.fetch_pull_requests(name, tap_remote_repo, state: "open")
pull_requests = GitHub.fetch_pull_requests(name, tap_remote_repo, state: state, version: version)
if pull_requests.try(:any?)
pull_requests = pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ")
end
Expand Down Expand Up @@ -248,8 +248,13 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
repology_latest
end.presence

pull_requests = if !args.no_pull_requests? && (args.named.present? || new_version)
retrieve_pull_requests(formula_or_cask, name)
open_pull_requests = if !args.no_pull_requests? && (args.named.present? || new_version)
retrieve_pull_requests(formula_or_cask, name, state: "open")
end.presence

closed_pull_requests = if !args.no_pull_requests? && !open_pull_requests && new_version.present?
# if we haven't already found open requests, try for an exact match across closed requests
retrieve_pull_requests(formula_or_cask, name, state: "closed", version: new_version)
end.presence

title_name = ambiguous_cask ? "#{name} (cask)" : name
Expand All @@ -265,7 +270,8 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
Current #{version_name}: #{current_version}
Latest livecheck version: #{livecheck_latest}
Latest Repology version: #{repology_latest}
Open pull requests: #{pull_requests || "none"}
Open pull requests: #{open_pull_requests || "none"}
Closed pull requests: #{closed_pull_requests || "none"}
EOS

return unless args.open_pr?
Expand All @@ -278,7 +284,8 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
end

return unless new_version
return if pull_requests
return if open_pull_requests
return if closed_pull_requests

system HOMEBREW_BREW_FILE, "bump-#{type}-pr", "--no-browse",
"--message=Created by `brew bump`", "--version=#{new_version}", name
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/utils/github.rb
View file
Open in desktop
Expand Up @@ -511,7 +511,7 @@ def check_for_duplicate_pull_requests(name, tap_remote_repo, state:, file:, args
return if pull_requests.blank?

duplicates_message = <<~EOS
These pull requests may be duplicates:
These #{state} pull requests may be duplicates:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just nice to have.

#{pull_requests.map { |pr| "#{pr["title"]} #{pr["html_url"]}" }.join("\n")}
EOS
error_message = "Duplicate PRs should not be opened. Use --force to override this error."
Expand Down