diff --git a/Library/Homebrew/dev-cmd/contributions.rb b/Library/Homebrew/dev-cmd/contributions.rb index 436afd0e7cd3c..c6eda85bf7c8b 100644 --- a/Library/Homebrew/dev-cmd/contributions.rb +++ b/Library/Homebrew/dev-cmd/contributions.rb @@ -163,13 +163,13 @@ def scan_repositories(repos, person, from:) puts "Determining contributions for #{person} on #{repo_full_name}..." if args.verbose? - author_commits, committer_commits = GitHub.count_repo_commits(repo_full_name, person, args, - max: MAX_REPO_COMMITS) + author_commits, committer_commits = GitHub.count_repo_commits(repo_full_name, person, + from:, to: args.to, max: MAX_REPO_COMMITS) data[repo] = { author: author_commits, committer: committer_commits, coauthorship: git_log_trailers_cmd(T.must(repo_path), person, "Co-authored-by", from:, to: args.to), - review: count_reviews(repo_full_name, person), + review: count_reviews(repo_full_name, person, from:, to: args.to), } end @@ -202,9 +202,12 @@ def git_log_trailers_cmd(repo_path, person, trailer, from:, to:) Utils.safe_popen_read(*cmd).lines.count { |l| l.include?(person) } end - sig { params(repo_full_name: String, person: String).returns(Integer) } - def count_reviews(repo_full_name, person) - GitHub.count_issues("", is: "pr", repo: repo_full_name, reviewed_by: person, review: "approved", args:) + sig { + params(repo_full_name: String, person: String, from: T.nilable(String), + to: T.nilable(String)).returns(Integer) + } + def count_reviews(repo_full_name, person, from:, to:) + GitHub.count_issues("", is: "pr", repo: repo_full_name, reviewed_by: person, review: "approved", from:, to:) rescue GitHub::API::ValidationFailedError if args.verbose? onoe "Couldn't search GitHub for PRs by #{person}. Their profile might be private. Defaulting to 0." diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index 56a047c5afc1f..2e4e4cb3329ec 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -99,48 +99,48 @@ it "counts commits authored by a user" do allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "author", {}, nil).and_return(five_shas) + .with("homebrew/cask", "user1", "author", nil, nil, nil).and_return(five_shas) allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "committer", {}, nil).and_return([]) + .with("homebrew/cask", "user1", "committer", nil, nil, nil).and_return([]) - expect(described_class.count_repo_commits("homebrew/cask", "user1", {})).to eq([5, 0]) + expect(described_class.count_repo_commits("homebrew/cask", "user1")).to eq([5, 0]) end it "counts commits committed by a user" do allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/core", "user1", "author", {}, nil).and_return([]) + .with("homebrew/core", "user1", "author", nil, nil, nil).and_return([]) allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/core", "user1", "committer", {}, nil).and_return(five_shas) + .with("homebrew/core", "user1", "committer", nil, nil, nil).and_return(five_shas) - expect(described_class.count_repo_commits("homebrew/core", "user1", {})).to eq([0, 5]) + expect(described_class.count_repo_commits("homebrew/core", "user1")).to eq([0, 5]) end it "calculates correctly when authored > committed with different shas" do allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "author", {}, nil).and_return(ten_shas) + .with("homebrew/cask", "user1", "author", nil, nil, nil).and_return(ten_shas) allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "committer", {}, nil).and_return(%w[1 2 3 4 5]) + .with("homebrew/cask", "user1", "committer", nil, nil, nil).and_return(%w[1 2 3 4 5]) - expect(described_class.count_repo_commits("homebrew/cask", "user1", {})).to eq([10, 5]) + expect(described_class.count_repo_commits("homebrew/cask", "user1")).to eq([10, 5]) end it "calculates correctly when committed > authored" do allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "author", {}, nil).and_return(five_shas) + .with("homebrew/cask", "user1", "author", nil, nil, nil).and_return(five_shas) allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/cask", "user1", "committer", {}, nil).and_return(ten_shas) + .with("homebrew/cask", "user1", "committer", nil, nil, nil).and_return(ten_shas) - expect(described_class.count_repo_commits("homebrew/cask", "user1", {})).to eq([5, 5]) + expect(described_class.count_repo_commits("homebrew/cask", "user1")).to eq([5, 5]) end it "deduplicates commits authored and committed by the same user" do allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/core", "user1", "author", {}, nil).and_return(five_shas) + .with("homebrew/core", "user1", "author", nil, nil, nil).and_return(five_shas) allow(described_class).to receive(:repo_commits_for_user) - .with("homebrew/core", "user1", "committer", {}, nil).and_return(five_shas) + .with("homebrew/core", "user1", "committer", nil, nil, nil).and_return(five_shas) # Because user1 authored and committed the same 5 commits. - expect(described_class.count_repo_commits("homebrew/core", "user1", {})).to eq([5, 0]) + expect(described_class.count_repo_commits("homebrew/core", "user1")).to eq([5, 0]) end end end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index d34ac967bd3a4..cecac086a2015 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -841,12 +841,12 @@ def self.multiple_short_commits_exist?(user, repo, commit) output[/^Status: (200)/, 1] != "200" end - def self.repo_commits_for_user(nwo, user, filter, args, max) + def self.repo_commits_for_user(nwo, user, filter, from, to, max) return if Homebrew::EnvConfig.no_github_api? params = ["#{filter}=#{user}"] - params << "since=#{DateTime.parse(args.from).iso8601}" if args.from - params << "until=#{DateTime.parse(args.to).iso8601}" if args.to + params << "since=#{DateTime.parse(from).iso8601}" if from.present? + params << "until=#{DateTime.parse(to).iso8601}" if to.present? commits = [] API.paginate_rest("#{API_URL}/repos/#{nwo}/commits", additional_query_params: params.join("&")) do |result| @@ -859,11 +859,11 @@ def self.repo_commits_for_user(nwo, user, filter, args, max) commits end - def self.count_repo_commits(nwo, user, args, max: nil) + def self.count_repo_commits(nwo, user, from: nil, to: nil, max: nil) odie "Cannot count commits, HOMEBREW_NO_GITHUB_API set!" if Homebrew::EnvConfig.no_github_api? - author_shas = repo_commits_for_user(nwo, user, "author", args, max) - committer_shas = repo_commits_for_user(nwo, user, "committer", args, max) + author_shas = repo_commits_for_user(nwo, user, "author", from, to, max) + committer_shas = repo_commits_for_user(nwo, user, "committer", from, to, max) return [0, 0] if author_shas.blank? && committer_shas.blank? author_count = author_shas.count