From e2968c788971e5a0fa5f7b31f4ad186477779cc4 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Mon, 21 May 2018 15:36:26 +0100 Subject: [PATCH] Various cleanup and fixes. Rename some variables and use existing regexes and a single variable. --- Library/Homebrew/dev-cmd/audit.rb | 12 +++++---- Library/Homebrew/utils/github.rb | 41 +++++++++++++++++-------------- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/Library/Homebrew/dev-cmd/audit.rb b/Library/Homebrew/dev-cmd/audit.rb index d7d54a9134..e915b62e63 100644 --- a/Library/Homebrew/dev-cmd/audit.rb +++ b/Library/Homebrew/dev-cmd/audit.rb @@ -137,16 +137,18 @@ module Homebrew end end - create_issue_failed = false - if new_formula && !new_formula_problem_lines.size.zero? && !GitHub.create_issue_no_op? + created_pr_comment = false + if new_formula && !new_formula_problem_lines.empty? begin - GitHub.create_issue_comment(new_formula_problem_lines.join("\n")) + if GitHub.create_issue_comment(new_formula_problem_lines.join("\n")) + created_pr_comment = true + end rescue *GitHub.api_errors - create_issue_failed = true + nil end end - if GitHub.create_issue_no_op? || create_issue_failed + unless created_pr_comment problem_count += new_formula_problem_lines.size puts new_formula_problem_lines.map { |s| " #{s}" } end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index 83496a45d9..75160c1edb 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -10,6 +10,8 @@ module GitHub CREATE_ISSUE_FORK_OR_PR_SCOPES = ["public_repo"].freeze ALL_SCOPES = (CREATE_GIST_SCOPES + CREATE_ISSUE_FORK_OR_PR_SCOPES).freeze ALL_SCOPES_URL = Formatter.url("https://github.com/settings/tokens/new?scopes=#{ALL_SCOPES.join(",")}&description=Homebrew").freeze + PR_ENV_KEY = "HOMEBREW_NEW_FORMULA_PULL_REQUEST_URL".freeze + PR_ENV = ENV[PR_ENV_KEY] Error = Class.new(RuntimeError) HTTPNotFoundError = Class.new(Error) @@ -254,14 +256,14 @@ module GitHub end def create_fork(repo) - url = "https://api.github.com/repos/#{repo}/forks" + url = "#{API_URL}/repos/#{repo}/forks" data = {} scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES open_api(url, data: data, scopes: scopes) end def create_pull_request(repo, title, head, base, body) - url = "https://api.github.com/repos/#{repo}/pulls" + url = "#{API_URL}/repos/#{repo}/pulls" data = { title: title, head: head, base: base, body: body } scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES open_api(url, data: data, scopes: scopes) @@ -292,27 +294,30 @@ module GitHub open_api(uri) { |json| json.fetch("items", []) } end - def create_issue_no_op? - !ENV.key?("HOMEBREW_GH_REPO") || !ENV.key?("HOMEBREW_NEW_FORMULA_PULL_REQUEST_URL") - end - def create_issue_comment(body) - repo = ENV["HOMEBREW_GH_REPO"] - pull_request = extract_pull_request_number - url = "https://api.github.com/repos/Homebrew/#{repo}/issues/#{pull_request}/comments" + return false unless PR_ENV + _, user, repo, pr = *PR_ENV.match(HOMEBREW_PULL_OR_COMMIT_URL_REGEX) + if !user || !repo || !pr + opoo <<-EOS.undent + #{PR_ENV_KEY} set but regex matched: + user: #{user.inspect}, repo: #{repo.inspect}, pr: #{pr.inspect} + EOS + return false + end + + url = "#{API_URL}/repos/#{user}/#{repo}/issues/#{pr}/comments" data = { "body" => body } - return if issue_comment_exists?(repo, pr_number, body) - scopes = CREATE_ISSUE_SCOPES + if issue_comment_exists?(user, repo, pr, body) + ohai "Skipping: identical comment exists on #{PR_ENV}." + return true + end + + scopes = CREATE_ISSUE_FORK_OR_PR_SCOPES open_api(url, data: data, scopes: scopes) end - def extract_pull_request_number - pattern = /#(\d+)$/ - ENV["HOMEBREW_NEW_FORMULA_PULL_REQUEST_URL"].match(pattern)[1] - end - - def issue_comment_exists?(repo, pull_reqest, body) - url = "https://api.github.com/repos/Homebrew/#{repo}/issues/#{pull_reqest}/comments" + def issue_comment_exists?(user, repo, pr, body) + url = "#{API_URL}/repos/#{user}/#{repo}/issues/#{pr}/comments" comments = open_api(url) return unless comments comments.any? { |comment| comment["body"].eql?(body) }