rubocop: Remove the final Naming/MethodParameterName exceptions: pr

- Core RuboCop didn't want this shortening upstreamed, but that's OK!
This commit is contained in:
Issy Long 2023-03-23 23:41:44 +00:00
parent 78feddce94
commit 61dc026fcc
No known key found for this signature in database
GPG Key ID: 8247C390DADC67D4
3 changed files with 37 additions and 36 deletions

View File

@ -182,15 +182,10 @@ Naming/MethodName:
AllowedPatterns: AllowedPatterns:
- '\A(fetch_)?HEAD\?\Z' - '\A(fetch_)?HEAD\?\Z'
# TODO: remove all of these
Naming/MethodParameterName: Naming/MethodParameterName:
inherit_mode: inherit_mode:
merge: merge:
- AllowedNames - 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, # Both styles are used depending on context,
# e.g. `sha256` and `something_countable_1`. # e.g. `sha256` and `something_countable_1`.

View File

@ -82,19 +82,19 @@ module Homebrew
[subject, body, trailers] [subject, body, trailers]
end 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) 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. # This is a tap pull request and approving reviewers should also sign-off.
tap = Tap.from_path(path) 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"]}>" "Signed-off-by: #{r["name"]} <#{r["email"]}>"
end end
trailers = trailers.lines.concat(review_trailers).map(&:strip).uniq.join("\n") trailers = trailers.lines.concat(review_trailers).map(&:strip).uniq.join("\n")
# Append the close message as well, unless the commit body already includes it. # 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 body += "\n\n#{close_message}" unless body.include? close_message
end end
@ -288,26 +288,26 @@ module Homebrew
raise raise
end 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? if args.dry_run?
puts <<~EOS 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 merge-base HEAD FETCH_HEAD
git cherry-pick --ff --allow-empty $merge_base..FETCH_HEAD git cherry-pick --ff --allow-empty $merge_base..FETCH_HEAD
EOS EOS
return return
end 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 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?) Utils::Git.cherry_pick!(path, "--ff", "--allow-empty", *commits, verbose: args.verbose?, resolve: args.resolve?)
end 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? 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") return false if labels.include?("CI-syntax-only") || labels.include?("CI-no-bottles")
@ -352,7 +352,7 @@ module Homebrew
formulae + casks formulae + casks
end 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 odie "Credentials must be set to access the Artifacts API" if GitHub::API.credentials_type == :none
token = GitHub::API.credentials token = GitHub::API.credentials
@ -362,20 +362,26 @@ module Homebrew
# preferred over system `curl` and `tar` as this leverages the Homebrew # preferred over system `curl` and `tar` as this leverages the Homebrew
# cache to avoid repeated downloads of (possibly large) bottles. # cache to avoid repeated downloads of (possibly large) bottles.
FileUtils.chdir dir do 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.fetch
downloader.stage downloader.stage
end end
end end
def self.pr_check_conflicts(repo, pr) def self.pr_check_conflicts(repo, pull_request)
long_build_pr_files = GitHub.issues( long_build_pr_files = GitHub.issues(
repo: repo, state: "open", labels: "no long build conflict", repo: repo, state: "open", labels: "no long build conflict",
).each_with_object({}) do |long_build_pr, hash| ).each_with_object({}) do |long_build_pr, hash|
next unless long_build_pr.key?("pull_request") next unless long_build_pr.key?("pull_request")
number = long_build_pr["number"] 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| GitHub.get_pull_request_changed_files(repo, number).each do |file|
key = file["filename"] key = file["filename"]
@ -386,7 +392,7 @@ module Homebrew
return if long_build_pr_files.blank? 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| conflicts = this_pr_files.each_with_object({}) do |file, hash|
filename = file["filename"] filename = file["filename"]
@ -463,7 +469,7 @@ module Homebrew
autosquash!(original_commit, tap: tap, autosquash!(original_commit, tap: tap,
verbose: args.verbose?, resolve: args.resolve?, reason: args.message) verbose: args.verbose?, resolve: args.resolve?, reason: args.message)
end 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 end
unless formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args) unless formulae_need_bottles?(tap, original_commit, user, repo, pr, args: args)

View File

@ -15,10 +15,10 @@ module GitHub
include SystemCommand::Mixin include SystemCommand::Mixin
def self.check_runs(repo: nil, commit: nil, pr: nil) def self.check_runs(repo: nil, commit: nil, pull_request: nil)
if pr if pull_request
repo = pr.fetch("base").fetch("repo").fetch("full_name") repo = pull_request.fetch("base").fetch("repo").fetch("full_name")
commit = pr.fetch("head").fetch("sha") commit = pull_request.fetch("head").fetch("sha")
end end
API.open_rest(url_to("repos", repo, "commits", commit, "check-runs")) API.open_rest(url_to("repos", repo, "commits", commit, "check-runs"))
@ -194,10 +194,10 @@ module GitHub
json.fetch("total_count", 0) json.fetch("total_count", 0)
end end
def self.approved_reviews(user, repo, pr, commit: nil) def self.approved_reviews(user, repo, pull_request, commit: nil)
query = <<~EOS query = <<~EOS
{ repository(name: "#{repo}", owner: "#{user}") { { repository(name: "#{repo}", owner: "#{user}") {
pullRequest(number: #{pr}) { pullRequest(number: #{pull_request}) {
reviews(states: APPROVED, first: 100) { reviews(states: APPROVED, first: 100) {
nodes { nodes {
author { 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) API.open_rest(url, data_binary_path: local_file, request_method: :POST, scopes: CREATE_ISSUE_FORK_OR_PR_SCOPES)
end 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 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. # 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 = { variables = {
user: user, user: user,
repo: repo, repo: repo,
pr: pr.to_i, pr: pull_request.to_i,
} }
result = API.open_graphql(query, variables: variables, scopes: scopes) result = API.open_graphql(query, variables: variables, scopes: scopes)
@ -339,7 +339,7 @@ module GitHub
[] []
end end
[check_suite, user, repo, pr, workflow_id, scopes, artifact_name] [check_suite, user, repo, pull_request, workflow_id, scopes, artifact_name]
end end
def self.get_artifact_url(workflow_array) def self.get_artifact_url(workflow_array)
@ -542,8 +542,8 @@ module GitHub
end end
end end
def self.get_pull_request_changed_files(tap_remote_repo, pr) def self.get_pull_request_changed_files(tap_remote_repo, pull_request)
API.open_rest(url_to("repos", tap_remote_repo, "pulls", pr, "files")) API.open_rest(url_to("repos", tap_remote_repo, "pulls", pull_request, "files"))
end end
def self.forked_repo_info!(tap_remote_repo, org: nil) def self.forked_repo_info!(tap_remote_repo, org: nil)
@ -656,8 +656,8 @@ module GitHub
end end
end end
def self.pull_request_commits(user, repo, pr, per_page: 100) def self.pull_request_commits(user, repo, pull_request, per_page: 100)
pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pr)) pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pull_request))
commits_api = pr_data["commits_url"] commits_api = pr_data["commits_url"]
commit_count = pr_data["commits"] commit_count = pr_data["commits"]
commits = [] commits = []
@ -677,8 +677,8 @@ module GitHub
end end
end end
def self.pull_request_labels(user, repo, pr) def self.pull_request_labels(user, repo, pull_request)
pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pr)) pr_data = API.open_rest(url_to("repos", user, repo, "pulls", pull_request))
pr_data["labels"].map { |label| label["name"] } pr_data["labels"].map { |label| label["name"] }
end end