Skip to content

Commit

Permalink
Merge pull request #17383 from Homebrew/fix-contributions-date-limiting
Browse files Browse the repository at this point in the history
  • Loading branch information
@carlocab
carlocab committed May 28, 2024
2 parents 92adf6a + 1f9c764 commit cd65109
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 27 deletions.
15 changes: 9 additions & 6 deletions Library/Homebrew/dev-cmd/contributions.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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."
Expand Down
30 changes: 15 additions & 15 deletions Library/Homebrew/test/utils/github_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 6 additions & 6 deletions Library/Homebrew/utils/github.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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|
Expand All @@ -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
Expand Down

0 comments on commit cd65109

Please sign in to comment.