diff --git a/Library/Homebrew/dev-cmd/contributions.rb b/Library/Homebrew/dev-cmd/contributions.rb index 9fe19f8b59..ffbb7fafd6 100755 --- a/Library/Homebrew/dev-cmd/contributions.rb +++ b/Library/Homebrew/dev-cmd/contributions.rb @@ -171,14 +171,9 @@ module Homebrew puts "Determining contributions for #{person} on #{repo_full_name}..." if args.verbose? - commits_authored = GitHub.repo_commit_count_for_user(repo_full_name, person, "author", args) - commits_committed = GitHub.repo_commit_count_for_user(repo_full_name, person, "committer", args) - # Only count committers where the author is not the same person. Avoid negative numbers or subtracting zero. - commits_committed = commits_authored - commits_committed if commits_authored > commits_committed - data[repo] = { - author: commits_authored, - committer: commits_committed, + author: GitHub.count_repo_commits(repo_full_name, person, "author", args), + committer: GitHub.count_repo_commits(repo_full_name, person, "committer", args), coauthorships: git_log_trailers_cmd(T.must(repo_path), person, "Co-authored-by", args), reviews: GitHub.count_issues( "", diff --git a/Library/Homebrew/test/utils/github_spec.rb b/Library/Homebrew/test/utils/github_spec.rb index da72e7e90c..cc8765e1f8 100644 --- a/Library/Homebrew/test/utils/github_spec.rb +++ b/Library/Homebrew/test/utils/github_spec.rb @@ -93,4 +93,56 @@ describe GitHub do expect(described_class.pull_request_commits("Homebrew", "legacy-homebrew", 50678, per_page: 1)).to eq(hashes) end end + + describe "::count_repo_commits" do + let(:five_shas) { %w[abcdef ghjkl mnop qrst uvwxyz] } + let(:ten_shas) { %w[abcdef ghjkl mnop qrst uvwxyz fedcba lkjhg ponm tsrq zyxwvu] } + + it "counts commits authored by a user" do + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/cask", "user1", "author", {}).and_return(five_shas) + + expect(described_class.count_repo_commits("homebrew/cask", "user1", "author", {})).to eq(5) + end + + it "counts commits committed by a user" do + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/core", "user1", "author", {}).and_return([]) + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/core", "user1", "committer", {}).and_return(five_shas) + + expect(described_class.count_repo_commits("homebrew/core", "user1", "committer", {})).to eq(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", {}).and_return(ten_shas) + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/cask", "user1", "committer", {}).and_return(%w[1 2 3 4 5]) + + expect(described_class.count_repo_commits("homebrew/cask", "user1", "author", {})).to eq(10) + expect(described_class.count_repo_commits("homebrew/cask", "user1", "committer", {})).to eq(5) + end + + it "calculates correctly when committed > authored" do + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/cask", "user1", "author", {}).and_return(five_shas) + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/cask", "user1", "committer", {}).and_return(ten_shas) + + expect(described_class.count_repo_commits("homebrew/cask", "user1", "author", {})).to eq(5) + expect(described_class.count_repo_commits("homebrew/cask", "user1", "committer", {})).to eq(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", {}).and_return(five_shas) + allow(described_class).to receive(:repo_commits_for_user) + .with("homebrew/core", "user1", "committer", {}).and_return(five_shas) + + # Because user1 authored and committed the same 5 commits. + expect(described_class.count_repo_commits("homebrew/core", "user1", "author", {})).to eq(5) + expect(described_class.count_repo_commits("homebrew/core", "user1", "committer", {})).to eq(0) + end + end end diff --git a/Library/Homebrew/utils/github.rb b/Library/Homebrew/utils/github.rb index d4bf5b7027..01ed527f82 100644 --- a/Library/Homebrew/utils/github.rb +++ b/Library/Homebrew/utils/github.rb @@ -717,17 +717,30 @@ module GitHub output[/^Status: (200)/, 1] != "200" end - def repo_commit_count_for_user(nwo, user, filter, args) + def repo_commits_for_user(nwo, user, filter, args) 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 - commits = 0 + commits = [] API.paginate_rest("#{API_URL}/repos/#{nwo}/commits", additional_query_params: params.join("&")) do |result| - commits += result.length + commits.concat(result.map { |c| c["sha"] }) end commits end + + def count_repo_commits(nwo, user, filter, args) + return if Homebrew::EnvConfig.no_github_api? + + author_shas = repo_commits_for_user(nwo, user, "author", args) + return author_shas.count if filter == "author" + + committer_shas = repo_commits_for_user(nwo, user, "committer", args) + return 0 if committer_shas.empty? + + # Only count commits where the author and committer are different. + committer_shas.difference(author_shas).count + end end