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: Stats for all maintainers #14722

Merged

Conversation

issyl0
Copy link
Member

@issyl0 issyl0 commented Feb 20, 2023

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

  • Part of Enhance brew contributions. #13642.
  • With brew contributions maintainers, this will output a list of stats (across the specified time period, or all time) for people in the "maintainers" team on GitHub.
  • This assumes that their Git committer details are the same as their name is set to on GitHub. There's a TODO here to work out how to do this better.
  • Show an error message if trying to generate a CSV for the full maintainer list, since I haven't worked out how to best show all of that info yet (or even how best to show only the totals across everything for every user) in that format.

@BrewTestBot
Copy link
Member

Review period will end on 2023-02-21 at 13:19:32 UTC.

@BrewTestBot BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 20, 2023
@issyl0
Copy link
Member Author

issyl0 commented Feb 20, 2023

  • This assumes that their Git committer details are the same as their name is set to on GitHub. There's a TODO here to work out how to do this better.

We can use the GitHub search API to search commits in a repo for a username, but the search APIs have some quite strict rate limiting I think?

There's also the commits REST API, so that's a better solution with better rate limits: https://docs.github.com/en/rest/commits/commits?apiVersion=2022-11-28#list-commits--parameters.

Also we don't need to tap all the repos if we get the data from GitHub. 🎉 I will figure all this out this week.

@issyl0
Copy link
Member Author

issyl0 commented Feb 20, 2023

The GitHub commits API doesn't support querying by trailers (Co-authored-by, Signed-off-by) or by committer for the other desired feature, splitting out commit authors and commit committers since they can be different. I've asked if these things will ever be available. Who knows, maybe I'll be able to write it myself!

@MikeMcQuaid
Copy link
Member

@issyl0 As a potential hack around this it just occurred to me: almost all of those cases are going to be ones where another maintainer was either the author or committer so: you might be able to do a maintainer-wide query and then filter commit messages after the fact.

@BrewTestBot BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Feb 21, 2023
@BrewTestBot
Copy link
Member

Review period ended.

@issyl0 issyl0 force-pushed the contributions-all-maintainers-sentence branch 2 times, most recently from 9d2dcae to 9abf469 Compare February 22, 2023 16:02
@issyl0
Copy link
Member Author

issyl0 commented Feb 22, 2023

I updated this so that just the bare brew contributions gets contributions for all maintainers.

- With `brew contributions`, this will output a list of stats
  (across the specified time period, or all time) for people in the
  "maintainers" team on GitHub.
- Add a `--user` flag for getting stats for a specific user (either
  username, name or email address).
- This assumes that their Git committer details are the same as their name is
  set to on GitHub.
- Show an error message if trying to generate a CSV for the full maintainer
  list, since I haven't worked out how to best show all of that info yet (or
  even how best to show only the totals across everything for every user) in
  that format.
@issyl0 issyl0 force-pushed the contributions-all-maintainers-sentence branch from 9abf469 to 214110f Compare February 22, 2023 16:06
@issyl0 issyl0 marked this pull request as ready for review February 22, 2023 16:06
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

This is great, thanks!

@MikeMcQuaid MikeMcQuaid merged commit b4e9f33 into Homebrew:master Feb 22, 2023
@MikeMcQuaid
Copy link
Member

Thanks again @issyl0!

@issyl0 issyl0 deleted the contributions-all-maintainers-sentence branch February 22, 2023 16:21
@github-actions github-actions bot added the outdated PR was locked due to age label Mar 25, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants