Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dev-cmd/contributions: Count PR reviews since they're super important #14813

Merged
merged 4 commits into from Feb 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 11 additions & 5 deletions Library/Homebrew/dev-cmd/contributions.rb
Expand Up @@ -108,7 +108,7 @@ def time_period(args)
sig { params(totals: Hash).returns(String) }
def generate_maintainers_csv(totals)
CSV.generate do |csv|
csv << %w[user repo commits coauthorships signoffs total]
csv << %w[user repo commits coauthorships signoffs reviews total]

totals.each do |user, total|
csv << grand_total_row(user, total)
Expand All @@ -119,14 +119,15 @@ def generate_maintainers_csv(totals)
sig { params(user: String, results: Hash, grand_total: Hash).returns(String) }
def generate_csv(user, results, grand_total)
CSV.generate do |csv|
csv << %w[user repo commits coauthorships signoffs total]
csv << %w[user repo commits coauthorships signoffs reviews total]
results.each do |repo, counts|
csv << [
user,
repo,
counts[:commits],
counts[:coauthorships],
counts[:signoffs],
counts[:reviews],
counts.values.sum,
]
end
Expand All @@ -142,6 +143,7 @@ def grand_total_row(user, grand_total)
grand_total[:commits],
grand_total[:coauthorships],
grand_total[:signoffs],
grand_total[:reviews],
grand_total.values.sum,
]
end
Expand Down Expand Up @@ -173,6 +175,7 @@ def scan_repositories(repos, person, args)
commits: GitHub.repo_commit_count_for_user(repo_full_name, person, args),
coauthorships: git_log_trailers_cmd(T.must(repo_path), person, "Co-authored-by", args),
signoffs: git_log_trailers_cmd(T.must(repo_path), person, "Signed-off-by", args),
reviews: GitHub.count_issues("", is: "pr", repo: repo_full_name, reviewed_by: person),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely wouldn't have been blocking: is there any way to query the specific type of reviews?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A query like is:pr reviewed-by:#{user} review:approved works to get approvals by the user, but we can't combine review:approved and review:changes_requested. So I thought that just reviewed-by:#{user} was safest to encompass everything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@issyl0 I'd consider filtering this to just review:approved as it'll get fewer results, requires write access (i.e. "maintainer duties") and is required to move things forward. Thoughts?

}
end

Expand All @@ -181,16 +184,19 @@ def scan_repositories(repos, person, args)

sig { params(results: Hash).returns(Hash) }
def total(results)
totals = { commits: 0, coauthorships: 0, signoffs: 0 }
totals = { commits: 0, coauthorships: 0, signoffs: 0, reviews: 0 }

# {"brew"=>{:commits=>9,:coauthorships=>6,:signoffs=>3},"core"=>{:commits=>15,:coauthorships=>10,:signoffs=>5}}
# {
# "brew"=>{:commits=>9,:coauthorships=>6,:signoffs=>3,:reviews=>1},
# "core"=>{:commits=>15,:coauthorships=>10,:signoffs=>5,:reviews=>2}
# }
results.each_value do |counts|
counts.each do |kind, count|
totals[kind] += count
end
end

totals # {:commits=>24,:coauthorships=>16,signoffs=>8}
totals # {:commits=>24,:coauthorships=>16,:signoffs=>8,:reviews=>3}
end

sig { params(repo_path: Pathname, person: String, trailer: String, args: Homebrew::CLI::Args).returns(Integer) }
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/test/search_spec.rb
Expand Up @@ -39,7 +39,7 @@
],
}

allow(GitHub::API).to receive(:open_rest).and_yield(json_response)
allow(GitHub::API).to receive(:open_rest).and_return(json_response)

expect(described_class.search_taps("some-formula"))
.to match(formulae: ["homebrew/foo/some-formula"], casks: ["homebrew/bar/some-cask"])
Expand Down
29 changes: 25 additions & 4 deletions Library/Homebrew/utils/github.rb
Expand Up @@ -37,7 +37,11 @@ def issues(repo:, **filters)
end

def search_issues(query, **qualifiers)
search("issues", query, **qualifiers)
search_results_items("issues", query, **qualifiers)
end

def count_issues(query, **qualifiers)
search_results_count("issues", query, **qualifiers)
end

def create_gist(files, description, private:)
Expand All @@ -57,7 +61,14 @@ def repository(user, repo)
end

def search_code(repo: nil, user: "Homebrew", path: ["Formula", "Casks", "."], filename: nil, extension: "rb")
matches = search("code", user: user, path: path, filename: filename, extension: extension, repo: repo)
matches = search_results_items(
"code",
user: user,
path: path,
filename: filename,
extension: extension,
repo: repo,
)
return matches if matches.blank?

matches.map do |match|
Expand Down Expand Up @@ -163,7 +174,7 @@ def search_query_string(*main_params, **qualifiers)
params = main_params

params += qualifiers.flat_map do |key, value|
Array(value).map { |v| "#{key}:#{v}" }
Array(value).map { |v| "#{key.to_s.tr("_", "-")}:#{v}" }
end

"q=#{URI.encode_www_form_component(params.join(" "))}&per_page=100"
Expand All @@ -176,7 +187,17 @@ def url_to(*subroutes)
def search(entity, *queries, **qualifiers)
uri = url_to "search", entity
uri.query = search_query_string(*queries, **qualifiers)
API.open_rest(uri) { |json| json.fetch("items", []) }
API.open_rest(uri)
end

def search_results_items(entity, *queries, **qualifiers)
json = search(entity, *queries, **qualifiers)
json.fetch("items", [])
end

def search_results_count(entity, *queries, **qualifiers)
json = search(entity, *queries, **qualifiers)
json.fetch("total_count", 0)
end

def approved_reviews(user, repo, pr, commit: nil)
Expand Down