From a89457fcb9a976f23a03e958b2d0c74b08a25081 Mon Sep 17 00:00:00 2001 From: Sam Ford <1584702+samford@users.noreply.github.com> Date: Sun, 24 Nov 2024 10:01:48 -0500 Subject: [PATCH] bump: skip PR checking when up to date `brew bump` will check for PRs related to a package even if the package version and livecheck version are the same. We're presumably only interested in related PRs when the livecheck version differs, so we can reduce GitHub API requests by skipping the check(s) when the versions are equal. We use `bump` in `autobump` workflows, so this should help with recent rate limiting issues (e.g., 3203 out of 3426 autobumped formulae were up to date in a recent run). This also reworks the output for duplicate PRs, making it clear when `bump` skipped checking PRs (as printing "none" would suggest that PRs were checked) and only printing the "Maybe duplicate" information when checked. This makes it a little easier to understand what `bump` has done internally from the output. --- Library/Homebrew/bump_version_parser.rb | 7 +++ Library/Homebrew/dev-cmd/bump.rb | 47 ++++++++++++------- .../Homebrew/test/bump_version_parser_spec.rb | 35 ++++++++++++++ 3 files changed, 73 insertions(+), 16 deletions(-) diff --git a/Library/Homebrew/bump_version_parser.rb b/Library/Homebrew/bump_version_parser.rb index cc47324e32..40c23e4b0d 100644 --- a/Library/Homebrew/bump_version_parser.rb +++ b/Library/Homebrew/bump_version_parser.rb @@ -48,5 +48,12 @@ module Homebrew def blank? @general.blank? && @arm.blank? && @intel.blank? end + + sig { params(other: T.untyped).returns(T::Boolean) } + def ==(other) + return false unless other.is_a?(BumpVersionParser) + + (general == other.general) && (arm == other.arm) && (intel == other.intel) + end end end diff --git a/Library/Homebrew/dev-cmd/bump.rb b/Library/Homebrew/dev-cmd/bump.rb index 771d4c5644..1491e4dc7e 100644 --- a/Library/Homebrew/dev-cmd/bump.rb +++ b/Library/Homebrew/dev-cmd/bump.rb @@ -277,8 +277,9 @@ module Homebrew odebug "Error fetching pull requests for #{formula_or_cask} #{name}: #{e}" nil end + return if pull_requests.blank? - pull_requests&.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }&.join(", ") + pull_requests.map { |pr| "#{pr["title"]} (#{Formatter.url(pr["html_url"])})" }.join(", ") end sig { @@ -373,13 +374,17 @@ module Homebrew new_version.general.to_s end - duplicate_pull_requests = unless args.no_pull_requests? - retrieve_pull_requests(formula_or_cask, name, version: pull_request_version) - end.presence + if !args.no_pull_requests? && (new_version != current_version) + duplicate_pull_requests = retrieve_pull_requests( + formula_or_cask, + name, + version: pull_request_version, + ) - maybe_duplicate_pull_requests = if !args.no_pull_requests? && duplicate_pull_requests.blank? - retrieve_pull_requests(formula_or_cask, name) - end.presence + maybe_duplicate_pull_requests = if duplicate_pull_requests.nil? + retrieve_pull_requests(formula_or_cask, name) + end + end VersionBumpInfo.new( type:, @@ -411,9 +416,7 @@ module Homebrew repology_latest = version_info.repology_latest # Check if all versions are equal - versions_equal = [:arm, :intel, :general].all? do |key| - current_version.send(key) == new_version.send(key) - end + versions_equal = (new_version == current_version) title_name = ambiguous_cask ? "#{name} (cask)" : name title = if (repology_latest == current_version.general || !repology_latest.is_a?(Version)) && versions_equal @@ -439,8 +442,8 @@ module Homebrew end version_label = version_info.version_name - duplicate_pull_requests = version_info.duplicate_pull_requests.presence - maybe_duplicate_pull_requests = version_info.maybe_duplicate_pull_requests.presence + duplicate_pull_requests = version_info.duplicate_pull_requests + maybe_duplicate_pull_requests = version_info.maybe_duplicate_pull_requests ohai title puts <<~EOS @@ -457,10 +460,22 @@ module Homebrew #{outdated_synced_formulae.join(", ")}. EOS end - puts <<~EOS unless args.no_pull_requests? - Duplicate pull requests: #{duplicate_pull_requests || "none"} - Maybe duplicate pull requests: #{maybe_duplicate_pull_requests || "none"} - EOS + if !args.no_pull_requests? && !versions_equal + if duplicate_pull_requests + duplicate_pull_requests_text = duplicate_pull_requests + elsif maybe_duplicate_pull_requests + duplicate_pull_requests_text = "none" + maybe_duplicate_pull_requests_text = maybe_duplicate_pull_requests + else + duplicate_pull_requests_text = "none" + maybe_duplicate_pull_requests_text = "none" + end + + puts "Duplicate pull requests: #{duplicate_pull_requests_text}" + if maybe_duplicate_pull_requests_text + puts "Maybe duplicate pull requests: #{maybe_duplicate_pull_requests_text}" + end + end return unless args.open_pr? diff --git a/Library/Homebrew/test/bump_version_parser_spec.rb b/Library/Homebrew/test/bump_version_parser_spec.rb index 2e47b14868..578d723e86 100644 --- a/Library/Homebrew/test/bump_version_parser_spec.rb +++ b/Library/Homebrew/test/bump_version_parser_spec.rb @@ -69,4 +69,39 @@ RSpec.describe Homebrew::BumpVersionParser do end end end + + describe "#==" do + let(:new_version) { described_class.new(general: general_version, arm: arm_version, intel: intel_version) } + + context "when other object is not a `BumpVersionParser`" do + it "returns false" do + expect(new_version == Version.new("1.2.3")).to be(false) + end + end + + context "when comparing objects with equal versions" do + it "returns true" do + same_version = described_class.new(general: general_version, arm: arm_version, intel: intel_version) + expect(new_version == same_version).to be(true) + end + end + + context "when comparing objects with different versions" do + it "returns false" do + different_general_version = described_class.new(general: "3.2.1", arm: arm_version, intel: intel_version) + different_arm_version = described_class.new(general: general_version, arm: "4.3.2", intel: intel_version) + different_intel_version = described_class.new(general: general_version, arm: arm_version, intel: "5.4.3") + different_general_arm_versions = described_class.new(general: "3.2.1", arm: "4.3.2", intel: intel_version) + different_arm_intel_versions = described_class.new(general: general_version, arm: "4.3.2", intel: "5.4.3") + different_general_intel_versions = described_class.new(general: "3.2.1", arm: arm_version, intel: "5.4.3") + + expect(new_version == different_general_version).to be(false) + expect(new_version == different_arm_version).to be(false) + expect(new_version == different_intel_version).to be(false) + expect(new_version == different_general_arm_versions).to be(false) + expect(new_version == different_arm_intel_versions).to be(false) + expect(new_version == different_general_intel_versions).to be(false) + end + end + end end