From 90309e5f42a9c60c6040d9d4ee0401fa413d1c7e Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Sat, 27 Jun 2020 22:01:51 +1000 Subject: [PATCH 1/3] github: fetch approved reviews for a pull request --- Library/Homebrew/test/utils/github_spec.rb | 7 ++++ Library/Homebrew/utils/github.rb | 46 ++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index 939ac6de5b..354c2585c4 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -42,6 +42,13 @@ describe GitHub do end end + describe "::approved_reviews", :needs_network do + it "can get reviews for a pull request" do + reviews = subject.approved_reviews("Homebrew", "homebrew-core", 1, commit: "deadbeef") + expect(reviews).to eq([]) + end + end + describe "::get_artifact_url", :needs_network do it "fails to find a nonexistant workflow" do expect { diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 556089c091..eae33a8eaf 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -394,6 +394,52 @@ module GitHub open_api(uri) { |json| json.fetch("items", []) } end + def approved_reviews(user, repo, pr, commit: nil) + url = "https://api.github.com/graphql" + data = { + query: <<~EOS, + { repository(name: "#{repo}", owner: "#{user}") { + pullRequest(number: #{pr}) { + reviews(states: APPROVED, first: 100) { + nodes { + author { + ... on User { email login name databaseId } + ... on Organization { email login name databaseId } + } + authorAssociation + commit { oid } + } + } + } + } + } + EOS + } + result = open_api(url, data: data, request_method: "POST") + raise Error, result["errors"] if result["errors"].present? + + reviews = result["data"]["repository"]["pullRequest"]["reviews"]["nodes"] + + reviews.map do |r| + next if commit.present? && commit != r["commit"]["oid"] + next unless %w[MEMBER OWNER].include? r["authorAssociation"] + + email = if r["author"]["email"].empty? + "#{r["author"]["databaseId"]}+#{r["author"]["login"]}@users.noreply.github.com" + else + r["author"]["email"] + end + + name = r["author"]["name"] || r["author"]["login"] + + { + "email" => email, + "name" => name, + "login" => r["author"]["login"], + } + end.compact + end + def dispatch_event(user, repo, event, **payload) url = "#{API_URL}/repos/#{user}/#{repo}/dispatches" open_api(url, data: { event_type: event, client_payload: payload }, From bada8dd7597ddaae8c2b88d1b5debc5cf4898422 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Sat, 27 Jun 2020 23:00:05 +1000 Subject: [PATCH 2/3] pr-pull: add review approval trailers on signoff --- Library/Homebrew/dev-cmd/pr-pull.rb | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index c5f82ec116..24d7e4e0f9 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -73,13 +73,18 @@ module Homebrew end end - def signoff!(pr, path: ".") - message = Utils.popen_read "git", "-C", path, "log", "-1", "--pretty=%B" + def signoff!(pr, tap:) + message = Utils.popen_read "git", "-C", tap.path, "log", "-1", "--pretty=%B" subject = message.lines.first.strip # Skip the subject and separate lines that look like trailers (e.g. "Co-authored-by") # from lines that look like regular body text. trailers, body = message.lines.drop(1).partition { |s| s.match?(/^[a-z-]+-by:/i) } + + # Approving reviewers also sign-off on merge + trailers += GitHub.approved_reviews(tap.user, "homebrew-#{tap.repo}", pr).map do |r| + "Signed-off-by: #{r["name"]} <#{r["email"]}>\n" + end trailers = trailers.uniq.join.strip body = body.join.strip.gsub(/\n{3,}/, "\n\n") @@ -90,7 +95,7 @@ module Homebrew if Homebrew.args.dry_run? puts "git commit --amend --signoff -m $message" else - safe_system "git", "-C", path, "commit", "--amend", "--signoff", "--allow-empty", "-q", "-m", new_message + safe_system "git", "-C", tap.path, "commit", "--amend", "--signoff", "--allow-empty", "-q", "-m", new_message end end @@ -232,7 +237,7 @@ module Homebrew cd dir do original_commit = Utils.popen_read("git", "-C", tap.path, "rev-parse", "HEAD").chomp cherry_pick_pr! pr, path: tap.path - signoff! pr, path: tap.path unless args.clean? + signoff! pr, tap: tap unless args.clean? unless args.no_upload? mirror_formulae(tap, original_commit, org: bintray_org, repo: mirror_repo, publish: !args.no_publish?) From 9cab9b7f39cb2c9d306c0dfcf33d7e1aa50c2ce3 Mon Sep 17 00:00:00 2001 From: Jonathan Chang Date: Mon, 29 Jun 2020 18:55:47 +1000 Subject: [PATCH 3/3] pr-pull: handle empty string cases Co-authored-by: Mike McQuaid --- Library/Homebrew/utils/github.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index eae33a8eaf..217332c733 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -424,13 +424,13 @@ module GitHub next if commit.present? && commit != r["commit"]["oid"] next unless %w[MEMBER OWNER].include? r["authorAssociation"] - email = if r["author"]["email"].empty? + email = if r["author"]["email"].blank? "#{r["author"]["databaseId"]}+#{r["author"]["login"]}@users.noreply.github.com" else r["author"]["email"] end - name = r["author"]["name"] || r["author"]["login"] + name = r["author"]["name"].presence || r["author"]["login"] { "email" => email,