diff --git a/Library/.rubocop.yml b/Library/.rubocop.yml index 2ba9ad3c19..f77b6a7560 100644 --- a/Library/.rubocop.yml +++ b/Library/.rubocop.yml @@ -182,15 +182,10 @@ Naming/MethodName: AllowedPatterns: - '\A(fetch_)?HEAD\?\Z' -# TODO: remove all of these Naming/MethodParameterName: inherit_mode: merge: - AllowedNames - AllowedNames: - [ - "pr", # TODO: Remove if https://github.com/rubocop/rubocop/pull/11690 is merged or we change the variable names. - ] # Both styles are used depending on context, # e.g. `sha256` and `something_countable_1`. diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index e8ec2195db..5bf66cbabb 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -82,19 +82,19 @@ module Homebrew [subject, body, trailers] end - def self.signoff!(path, pr: nil, dry_run: false) + def self.signoff!(path, pull_request: nil, dry_run: false) subject, body, trailers = separate_commit_message(path.git_commit_message) - if pr + if pull_request # This is a tap pull request and approving reviewers should also sign-off. tap = Tap.from_path(path) - review_trailers = GitHub.approved_reviews(tap.user, tap.full_name.split("/").last, pr).map do |r| + review_trailers = GitHub.approved_reviews(tap.user, tap.full_name.split("/").last, pull_request).map do |r| "Signed-off-by: #{r["name"]} <#{r["email"]}>" end trailers = trailers.lines.concat(review_trailers).map(&:strip).uniq.join("\n") # Append the close message as well, unless the commit body already includes it. - close_message = "Closes ##{pr}." + close_message = "Closes ##{pull_request}." body += "\n\n#{close_message}" unless body.include? close_message end @@ -288,26 +288,26 @@ module Homebrew raise end - def self.cherry_pick_pr!(user, repo, pr, args:, path: ".") + def self.cherry_pick_pr!(user, repo, pull_request, args:, path: ".") if args.dry_run? puts <<~EOS - git fetch --force origin +refs/pull/#{pr}/head + git fetch --force origin +refs/pull/#{pull_request}/head git merge-base HEAD FETCH_HEAD git cherry-pick --ff --allow-empty $merge_base..FETCH_HEAD EOS return end - commits = GitHub.pull_request_commits(user, repo, pr) + commits = GitHub.pull_request_commits(user, repo, pull_request) safe_system "git", "-C", path, "fetch", "--quiet", "--force", "origin", commits.last - ohai "Using #{commits.count} commit#{"s" unless commits.count == 1} from ##{pr}" + ohai "Using #{commits.count} commit#{"s" unless commits.count == 1} from ##{pull_request}" Utils::Git.cherry_pick!(path, "--ff", "--allow-empty", *commits, verbose: args.verbose?, resolve: args.resolve?) end - def self.formulae_need_bottles?(tap, original_commit, user, repo, pr, args:) + def self.formulae_need_bottles?(tap, original_commit, user, repo, pull_request, args:) return if args.dry_run? - labels = GitHub.pull_request_labels(user, repo, pr) + labels = GitHub.pull_request_labels(user, repo, pull_request) return false if labels.include?("CI-syntax-only") || labels.include?("CI-no-bottles") @@ -352,7 +352,7 @@ module Homebrew formulae + casks end - def self.download_artifact(url, dir, pr) + def self.download_artifact(url, dir, pull_request) odie "Credentials must be set to access the Artifacts API" if GitHub::API.credentials_type == :none token = GitHub::API.credentials @@ -362,20 +362,26 @@ module Homebrew # preferred over system `curl` and `tar` as this leverages the Homebrew # cache to avoid repeated downloads of (possibly large) bottles. FileUtils.chdir dir do - downloader = GitHubArtifactDownloadStrategy.new(url, "artifact", pr, curl_args: curl_args, secrets: [token]) + downloader = GitHubArtifactDownloadStrategy.new( + url, + "artifact", + pull_request, + curl_args: curl_args, + secrets: [token], + ) downloader.fetch downloader.stage end end - def self.pr_check_conflicts(repo, pr) + def self.pr_check_conflicts(repo, pull_request) long_build_pr_files = GitHub.issues( repo: repo, state: "open", labels: "no long build conflict", ).each_with_object({}) do |long_build_pr, hash| next unless long_build_pr.key?("pull_request") number = long_build_pr["number"] - next if number == pr.to_i + next if number == pull_request.to_i GitHub.get_pull_request_changed_files(repo, number).each do |file| key = file["filename"] @@ -386,7 +392,7 @@ module Homebrew return if long_build_pr_files.blank? - this_pr_files = GitHub.get_pull_request_changed_files(repo, pr) + this_pr_files = GitHub.get_pull_request_changed_files(repo, pull_request) conflicts = this_pr_files.each_with_object({}) do |file, hash| filename = file["filename"] @@ -463,7 +469,7 @@ module Homebrew autosquash!(original_commit, tap: tap, verbose: args.verbose?, resolve: args.resolve?, reason: args.message) end - signoff!(tap.path, pr: pr, dry_run: args.dry_run?) unless args.clean? + signoff!(tap.path, pull_request: pr, dry_run: args.dry_run?) unless args.clean? end unless formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 7429f59da9..9c13d8b0c3 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -15,10 +15,10 @@ module GitHub include SystemCommand::Mixin - def self.check_runs(repo: nil, commit: nil, pr: nil) - if pr - repo = pr.fetch("base").fetch("repo").fetch("full_name") - commit = pr.fetch("head").fetch("sha") + def self.check_runs(repo: nil, commit: nil, pull_request: nil) + if pull_request + repo = pull_request.fetch("base").fetch("repo").fetch("full_name") + commit = pull_request.fetch("head").fetch("sha") end API.open_rest(url_to("repos", repo, "commits", commit, "check-runs")) @@ -194,10 +194,10 @@ module GitHub json.fetch("total_count", 0) end - def self.approved_reviews(user, repo, pr, commit: nil) + def self.approved_reviews(user, repo, pull_request, commit: nil) query = <<~EOS { repository(name: "#{repo}", owner: "#{user}") { - pullRequest(number: #{pr}) { + pullRequest(number: #{pull_request}) { reviews(states: APPROVED, first: 100) { nodes { author { @@ -289,7 +289,7 @@ module GitHub API.open_rest(url, data_binary_path: local_file, request_method: :POST, scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES) end - def self.get_workflow_run(user, repo, pr, workflow_id: "tests.yml", artifact_name: "bottles") + def self.get_workflow_run(user, repo, pull_request, workflow_id: "tests.yml", artifact_name: "bottles") scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES # GraphQL unfortunately has no way to get the workflow yml name, so we need an extra REST call. @@ -326,7 +326,7 @@ module GitHub variables = { user: user, repo: repo, - pr: pr.to_i, + pr: pull_request.to_i, } result = API.open_graphql(query, variables: variables, scopes: scopes) @@ -339,7 +339,7 @@ module GitHub [] end - [check_suite, user, repo, pr, workflow_id, scopes, artifact_name] + [check_suite, user, repo, pull_request, workflow_id, scopes, artifact_name] end def self.get_artifact_url(workflow_array) @@ -542,8 +542,8 @@ module GitHub end end - def self.get_pull_request_changed_files(tap_remote_repo, pr) - API.open_rest(url_to("repos", tap_remote_repo, "pulls", pr, "files")) + def self.get_pull_request_changed_files(tap_remote_repo, pull_request) + API.open_rest(url_to("repos", tap_remote_repo, "pulls", pull_request, "files")) end def self.forked_repo_info!(tap_remote_repo, org: nil) @@ -656,8 +656,8 @@ module GitHub end end - def self.pull_request_commits(user, repo, pr, per_page: 100) - pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pr)) + def self.pull_request_commits(user, repo, pull_request, per_page: 100) + pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pull_request)) commits_api = pr_data["commits_url"] commit_count = pr_data["commits"] commits = [] @@ -677,8 +677,8 @@ module GitHub end end - def self.pull_request_labels(user, repo, pr) - pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pr)) + def self.pull_request_labels(user, repo, pull_request) + pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pull_request)) pr_data["labels"].map { |label| label["name"] } end