dev-cmd/contributions: Fix the date range behaviour
- This was broken (I did have a commit SHA for the breakage but I can't find it now) since `from` and `args.from` are different variables (one can be nil, the other has a default value). - So it was reporting very high counts because, despite the message, the `from` restriction was not being passed to `count_repo_commits`.
This commit is contained in:
parent
92adf6a0b7
commit
808cfda92d
@ -163,8 +163,8 @@ module Homebrew
|
||||
|
||||
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,
|
||||
|
@ -99,48 +99,48 @@ RSpec.describe GitHub do
|
||||
|
||||
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
|
||||
|
@ -841,12 +841,12 @@ module GitHub
|
||||
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 @@ module GitHub
|
||||
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
|
||||
|
Loading…
x
Reference in New Issue
Block a user