From 04f6f53b58460e63d184babbfa3c7470957b7a08 Mon Sep 17 00:00:00 2001 From: Bo Anderson Date: Sat, 3 Sep 2022 20:54:09 +0100 Subject: [PATCH] dev-cmd/pr-pull: avoid expensive search API calls --- Library/Homebrew/dev-cmd/pr-pull.rb | 18 +++++++++++------- Library/Homebrew/test/utils/github_spec.rb | 8 ++++---- Library/Homebrew/utils/github.rb | 10 ++++++++-- 3 files changed, 23 insertions(+), 13 deletions(-) diff --git a/Library/Homebrew/dev-cmd/pr-pull.rb b/Library/Homebrew/dev-cmd/pr-pull.rb index 9bac23c98c..c8015d3935 100644 --- a/Library/Homebrew/dev-cmd/pr-pull.rb +++ b/Library/Homebrew/dev-cmd/pr-pull.rb @@ -368,28 +368,32 @@ module Homebrew end end - def pr_check_conflicts(user, repo, pr) - long_build_pr_files = GitHub.search_issues( - "org:#{user}", repo: repo, state: "open", label: "\"no long build conflict\"" + def pr_check_conflicts(repo, pr) + 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 - GitHub.get_pull_request_changed_files("#{user}/#{repo}", number).each do |file| + GitHub.get_pull_request_changed_files(repo, number).each do |file| key = file["filename"] hash[key] ||= [] hash[key] << number end end - this_pr_files = GitHub.get_pull_request_changed_files("#{user}/#{repo}", pr) + return if long_build_pr_files.blank? + + this_pr_files = GitHub.get_pull_request_changed_files(repo, pr) conflicts = this_pr_files.each_with_object({}) do |file, hash| filename = file["filename"] next unless long_build_pr_files.key?(filename) long_build_pr_files[filename].each do |pr_number| - key = "#{user}/#{repo}/pull/#{pr_number}" + key = "#{repo}/pull/#{pr_number}" hash[key] ||= [] hash[key] << filename end @@ -440,7 +444,7 @@ module Homebrew opoo "Current branch is #{tap.path.git_branch}: do you need to pull inside #{tap.path.git_origin_branch}?" end - pr_check_conflicts(user, repo, pr) + pr_check_conflicts("#{user}/#{repo}", pr) ohai "Fetching #{tap} pull request ##{pr}" Dir.mktmpdir pr do |dir| diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index 8221b76b2f..daecb0f52a 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -15,19 +15,19 @@ describe GitHub do end end - describe "::query_string" do + describe "::search_query_string" do it "builds a query with the given hash parameters formatted as key:value" do - query = described_class.query_string(user: "Homebrew", repo: "brew") + query = described_class.search_query_string(user: "Homebrew", repo: "brew") expect(query).to eq("q=user%3AHomebrew+repo%3Abrew&per_page=100") end it "adds a variable number of top-level string parameters to the query when provided" do - query = described_class.query_string("value1", "value2", user: "Homebrew") + query = described_class.search_query_string("value1", "value2", user: "Homebrew") expect(query).to eq("q=value1+value2+user%3AHomebrew&per_page=100") end it "turns array values into multiple key:value parameters" do - query = described_class.query_string(user: ["Homebrew", "caskroom"]) + query = described_class.search_query_string(user: ["Homebrew", "caskroom"]) expect(query).to eq("q=user%3AHomebrew+user%3Acaskroom&per_page=100") end end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index d3d956bd55..5253b875b6 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -30,6 +30,12 @@ module GitHub API.open_rest(url_to("repos", repo, "check-runs"), data: data) end + def issues(repo:, **filters) + uri = url_to("repos", repo, "issues") + uri.query = URI.encode_www_form(filters) + API.open_rest(uri) + end + def search_issues(query, **qualifiers) search("issues", query, **qualifiers) end @@ -153,7 +159,7 @@ module GitHub API.open_rest(uri) { |json| json["private"] } end - def query_string(*main_params, **qualifiers) + def search_query_string(*main_params, **qualifiers) params = main_params params += qualifiers.flat_map do |key, value| @@ -169,7 +175,7 @@ module GitHub def search(entity, *queries, **qualifiers) uri = url_to "search", entity - uri.query = query_string(*queries, **qualifiers) + uri.query = search_query_string(*queries, **qualifiers) API.open_rest(uri) { |json| json.fetch("items", []) } end