Skip to content

Commit

Permalink
bump: skip PR checking when up to date
Browse files Browse the repository at this point in the history
`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.
  • Loading branch information
samford committed Nov 25, 2024
1 parent 8d30564 commit a89457f
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 16 deletions.
7 changes: 7 additions & 0 deletions Library/Homebrew/bump_version_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,5 +48,12 @@ def parse_cask_version(version)
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
47 changes: 31 additions & 16 deletions Library/Homebrew/dev-cmd/bump.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,8 +277,9 @@ def retrieve_pull_requests(formula_or_cask, name, version: nil)
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 {
Expand Down Expand Up @@ -373,13 +374,17 @@ def retrieve_versions_by_arch(formula_or_cask:, repositories:, name:)
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:,
Expand Down Expand Up @@ -411,9 +416,7 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
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
Expand All @@ -439,8 +442,8 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
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
Expand All @@ -457,10 +460,22 @@ def retrieve_and_display_info_and_open_pr(formula_or_cask, name, repositories, a
#{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?

Expand Down
35 changes: 35 additions & 0 deletions Library/Homebrew/test/bump_version_parser_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,4 +69,39 @@
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

0 comments on commit a89457f

Please sign in to comment.