dev-cmd/contributions: Retrieve a user's repo contributions over time #13603

issyl0 · 2022-07-24T21:27:06Z

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?

Before each AGM it's currently a manual process for a PLC member to search commit logs and GitHub to figure out who contributed to Homebrew, so who should remain a member.
I noticed that looking at commits for a user would not count Co-Authored-By, which happens a lot now there's an autosquash action on PRs in Homebrew/homebrew-core, say if someone fixed a formula's build or tests or whatever and then the PR got auto-merged.
Here's brew contributions that uses git log to be able to go back through all time or a specific time period (--from, --to). It's up to individual PLC discretion for "activity", but it does at least go some way to automating the data retrieval.
Example (I can use my username as --email because my username is in all of the email addresses that I use for committing to Homebrew):

$ brew contributions --email=issyl0 --repos=brew,core
Person issyl0 directly authored 732 commits and co-authored 31 commits to brew, core in all time.

Future enhancement ideas:

Ability to specify multiple email addresses for a user?
Add a --username= flag to specify a GitHub username (instead of/as well as an email). As we know, open source isn't all about code, but reviews count as well as they're extremely valuable with the number of PRs we get. We could get data on how many reviews a user has submitted on PRs. And maybe we could use --username to retrieve the user's email addresses from GitHub too, so that the user of this script doesn't need to know them.
Some other things I've not thought of yet. Ideas welcome!

BrewTestBot · 2022-07-24T21:27:21Z

Review period will end on 2022-07-26 at 00:00:00 UTC.

Library/Homebrew/dev-cmd/contributions.rb

Rylan12

Cool idea!

Is it possible to use the GitHub API so this doesn't require having things tapped locally?

Library/Homebrew/dev-cmd/contributions.rb

issyl0 · 2022-07-24T22:11:27Z

Is it possible to use the GitHub API so this doesn't require having things tapped locally?

Technically probably yes, but I dread to think how many API rate limits we'll hit for the commit searches across all the things.

I know the JSON stuff will mean that at some point we won't have all the repos tapped.

I'll look into it and try some things out. But I'm thinking "future enhancement"?

Rylan12 · 2022-07-24T22:14:05Z

Technically probably yes, but I dread to think how many API rate limits we'll hit for the commit searches across all the things.

Yeah, I was thinking that if there was an equivalent API endpoint for git log it would be doable, but otherwise not worth the effort.

I know the JSON stuff will mean that at some point we won't have all the repos tapped.

I mean yes, but developers will (mostly) need to have the repos tapped anyway for editing files and the like, so I'm not worried about this. Since it's a dev cmd, it's not a problem to just require the repo to be tapped.

issyl0 · 2022-07-24T22:39:18Z

Yeah, I was thinking that if there was an equivalent API endpoint for git log it would be doable, but otherwise not worth the effort.

There's the commits API, but it's maximum 100 commits per page. 😩

Since it's a dev cmd, it's not a problem to just require the repo to be tapped.

😅

Library/Homebrew/dev-cmd/contributions.rb

MikeMcQuaid · 2022-07-25T08:04:10Z

There's the commits API, but it's maximum 100 commits per page. 😩

@issyl0 there's an author query parameter you can use for filtering here. Need to check if it works for Co-Authored-By but, if so, that would work well. We don't need more than 100 commits per author.

MikeMcQuaid · 2022-07-25T08:06:25Z

Add a --username= flag to specify a GitHub username (instead of/as well as an email). As we know, open source isn't all about code, but reviews count as well as they're extremely valuable with the number of PRs we get. We could get data on how many reviews a user has submitted on PRs. And maybe we could use --username to retrieve the user's email addresses from GitHub too, so that the user of this script doesn't need to know them.

@issyl0 I think doing this by username would be better if possible; it's not always clear the correct email for a given user.

Some other things I've not thought of yet. Ideas welcome!

Ideally, we'd also be able to query e.g. ✅ or 🟥 reviews (i.e. those that require write access) to check those maintainer actions, too.

MikeMcQuaid

Looks great so far, nice work @issyl0!

Library/Homebrew/dev-cmd/contributions.rb

carlocab · 2022-07-25T16:45:22Z

Not sure if you want to handle this...

❯ brew contributions --repos=core BrewTestBot
Error: Failure while executing; `git\ -C\ /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core\ log\ --oneline\ --format=\'\%\(trailers:key=Co-authored-by:\)\'\ \|\ grep\ BrewTestBot` exited with 1. Here's the output:
❯ brew contributions --repos=core --debug BrewTestBot
Error: Failure while executing; `git\ -C\ /usr/local/Homebrew/Library/Taps/homebrew/homebrew-core\ log\ --oneline\ --format=\'\%\(trailers:key=Co-authored-by:\)\'\ \|\ grep\ BrewTestBot` exited with 1. Here's the output:
/usr/local/Homebrew/Library/Homebrew/utils/popen.rb:12:in `popen_read'
/usr/local/Homebrew/Library/Homebrew/utils/popen.rb:16:in `safe_popen_read'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/contributions.rb:98:in `git_log_coauthor_cmd'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/contributions.rb:54:in `block in contributions'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/contributions.rb:45:in `each'
/usr/local/Homebrew/Library/Homebrew/dev-cmd/contributions.rb:45:in `contributions'
/usr/local/Homebrew/Library/Homebrew/brew.rb:93:in `<main>'

BrewTestBot · 2022-07-26T00:06:03Z

Review period ended.

issyl0 · 2022-07-26T09:18:26Z

Marked this as draft since there's still a lot to do - thanks for all your comments! I'll finish this off by ~Thursday.

issyl0 · 2022-07-28T11:02:14Z

Not sure if you want to handle [git failures while scanning BrewTestBot's commits]...

@carlocab This doesn't happen anymore, that I can reproduce anyway. 🎉

Library/Homebrew/dev-cmd/contributions.rb

MikeMcQuaid

Looking great so far! Feature request, not needing all to be handled in this PR:

count both commit authors and commit committers
count "PRs reviewed ✅ or 🟥 by" (which implies write access)

Library/Homebrew/dev-cmd/contributions.rb

MikeMcQuaid · 2022-07-29T11:06:17Z

Library/Homebrew/dev-cmd/contributions.rb

+      flag "--to=",
+           description: "Date (ISO-8601 format) to stop searching contributions."
+
+      comma_array "--repos=",


Suggested change

comma_array "--repos=",

comma_array "--repositories=", "--repos",

comma_array doesn't take multiple names. Yet. I suppose I could fix that.

Library/Homebrew/dev-cmd/contributions.rb

issyl0 · 2022-07-31T21:23:14Z

TODO (not all in this PR, I'm hoping to ship this soon, once this is in I'll write up an issue for the things that are left 😅):

Replace email with username, if there's a way of finding all email addresses (linked to their GitHub account?) a username committed with to get accurate numbers. Or maybe we could do name?
Figure out a way to use the GitHub APIs rather than git log. There currently doesn't seem to be any support for Co-authored-by authorships.
Count both commit authors and commit committers.
Count "PRs reviewed ✅ or 🟥 by" (which implies write access).

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

- I got these with hash syntax because I couldn't figure out Sorbet, but there's `args.rbi` to add the CLI args methods to. Nice! - In doing this I realised that `--repositories` is required again, we no longer infer `--repositories=all` from no `--repositories` passed as we did in a previous version of this.

- This is apparently "more in-keeping with how we do required arguments (never requiring flags)" across Homebrew. - New usage: `brew contributions me@issyl0.co.uk all`, `brew contributions me@issyl0.co.uk brew,core`

- This is easier than mapping GitHub usernames to email addresses, when folks don't have email addresses always public on their GitHub profiles. Also, the users of this command (PLC members, other interested parties) don't have to remember folks' email addresses. - It also gives better data for people who've changed their name over the years, and who commit with multiple email addresses (personal and work). ``` ❯ brew contributions "Issy Long" all Person Issy Long directly authored 687 commits and co-authored 33 commits across all Homebrew repos in all time. ❯ brew contributions "Rylan Polster" all Person Rylan Polster directly authored 1747 commits and co-authored 133 commits across all Homebrew repos in all time. ❯ brew contributions me@issyl0.co.uk all Person me@issyl0.co.uk directly authored 711 commits and co-authored 25 commits across all Homebrew repos in all time. ❯ brew contributions "Mike McQuaid" all Person Mike McQuaid directly authored 26879 commits and co-authored 204 commits across all Homebrew repos in all time. ```

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

- Also stop skipping a "versions" repo. Since 0232610 the `Homebrew/homebrew-cask-versions` tap won't get mistaken for `Homebrew/homebrew-versions` tap.

- This doesn't require "all" to be specified as part of the command, it's the default, so usage is now just: ``` $ brew contributions "Issy Long" $ brew contributions "Issy Long" --repositories=brew,core $ brew contributions me@issyl0.co.uk --repositories=cask,bundle ``` - As we discussed in the PR review before, `comma_array` doesn't allow two names, so we can't (yet) do `comma_array "--repositories", "--repos"` like we can with `flag`. That's an enhancement for the future if we want to make the flags here less verbose. But now that "all" is the default, maybe less necessary.

issyl0 · 2022-08-03T16:13:50Z

@carlocab @Rylan12 Thanks for your thoughts and review thus far! ✨ Any more thoughts or can I ?

Rylan12

A few minor things

Rylan12 · 2022-08-03T16:24:14Z

Library/Homebrew/dev-cmd/contributions.rb

+  SUPPORTED_REPOS = [
+    %w[brew core cask],
+    OFFICIAL_CMD_TAPS.keys.map { |t| t.delete_prefix("homebrew/") },
+    OFFICIAL_CASK_TAPS.reject { |t| t == "cask" },


I don't think this needs to block this PR, but isn't OFFICIAL_CASK_TAPS still missing cask-drivers and cask-fonts?

Yeah, I don't know why those don't exist. Casks aren't my area of expertise. I'll do a separate PR to add them and see what folks say.

Rylan12 · 2022-08-03T16:26:37Z

Library/Homebrew/dev-cmd/contributions.rb

+
+      comma_array "--repositories",
+                  description: "Specify a comma-separated (no spaces) list of repositories to search. " \
+                               "Supported repositories: #{SUPPORTED_REPOS.join(", ")}. " \


Again, not really blocking but we could use #to_sentence here. Also, it might be nice to get backticks around the options so they stand out (unless you think that looks too chaotic or isn't worth the complexity)

Suggested change

"Supported repositories: #{SUPPORTED_REPOS.join(", ")}. " \

"Supported repositories: #{SUPPORTED_REPOS.map { |t| "`#{t}`" }.to_sentence}." \

looks pretty good

Library/Homebrew/dev-cmd/contributions.rb

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

Rylan12

Looks good to me! Nice work!

issyl0 · 2022-08-03T18:03:17Z

CodeCov is the only failing check now, and I've addressed all the immediate comments, so I'm going to merge this and write up an issue for the remaining things. Thanks for your reviews!

- This is needed for Homebrew#13603 because `Homebrew/homebrew-versions` is deprecated (legitimately), but `OFFICIAL_CASK_TAPS` spits out `versions` as a repo, which could then be interpreted as `Homebrew/versions` rather than `Homebrew/cask-versions` which it actually is.

MikeMcQuaid · 2022-08-04T10:37:36Z

Thanks @issyl0!

- This [came up](Homebrew#13603 (comment)) when developing `brew contributions`, the `--repositories` flag (comma array) is long to type. It turned out that unlike normal flags, `comma_array`s didn't allow multiple arg names. - Now in `brew contributions` (and potentially elsewhere, though I've not had a look) we can make the args easier to type by letting `--repositories` and `--repos` equate to the same thing.

BrewTestBot added the waiting for feedback Merging is blocked until sufficient time has passed for review label Jul 24, 2022

issyl0 commented Jul 24, 2022

View reviewed changes

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

issyl0 force-pushed the cmd-contributions branch from 9d0d60d to ec0f9a7 Compare July 24, 2022 21:50

Rylan12 reviewed Jul 24, 2022

View reviewed changes

issyl0 force-pushed the cmd-contributions branch from f083b94 to c22efba Compare July 24, 2022 22:36

issyl0 force-pushed the cmd-contributions branch from d65daca to 7ab7b65 Compare July 24, 2022 22:48

Rylan12 reviewed Jul 25, 2022

View reviewed changes

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

Rylan12 reviewed Jul 25, 2022

View reviewed changes

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

MikeMcQuaid reviewed Jul 25, 2022

View reviewed changes

carlocab reviewed Jul 25, 2022

View reviewed changes

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

BrewTestBot removed the waiting for feedback Merging is blocked until sufficient time has passed for review label Jul 26, 2022

BrewTestBot approved these changes Jul 26, 2022

View reviewed changes

issyl0 marked this pull request as draft July 26, 2022 09:17

issyl0 force-pushed the cmd-contributions branch from 974f8e9 to dcbf414 Compare July 28, 2022 10:52

Rylan12 reviewed Jul 28, 2022

View reviewed changes

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

Library/Homebrew/dev-cmd/contributions.rb Outdated Show resolved Hide resolved

MikeMcQuaid reviewed Jul 29, 2022

View reviewed changes

issyl0 force-pushed the cmd-contributions branch 2 times, most recently from bdd2d97 to fd57f9e Compare July 29, 2022 20:51

issyl0 marked this pull request as ready for review August 1, 2022 08:47

issyl0 and others added 10 commits August 3, 2022 16:53

dev-cmd/contributions: Use --repositories not --repos

f5d5778

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

dev-cmd/contributions: Improve SUPPORTED_REPOS array syntax

63a1a07

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>

dev-cmd/contributions: Properly auto-tap

5d312d2

dev-cmd/contributions: Use a named arg for required repositories

ae73f28

- This is apparently "more in-keeping with how we do required arguments (never requiring flags)" across Homebrew. - New usage: `brew contributions me@issyl0.co.uk all`, `brew contributions me@issyl0.co.uk brew,core`

dev-cmd/contributions: Start output with the name/email

5ecdf10

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

dev-cmd/contributions: Remove named arg names, unused

a7effbd

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

dev-cmd/contributions: Improve Sorbet typing

8c762d9

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

dev-cmd/contributions: Don't have cask in SUPPORTED_REPOS twice

3133166

- Also stop skipping a "versions" repo. Since 0232610 the `Homebrew/homebrew-cask-versions` tap won't get mistaken for `Homebrew/homebrew-versions` tap.

issyl0 force-pushed the cmd-contributions branch from 40b53e1 to 3133166 Compare August 3, 2022 15:56

issyl0 requested review from Rylan12 and carlocab August 3, 2022 16:13

Rylan12 reviewed Aug 3, 2022

View reviewed changes

dev-cmd/contributions: Better usage string & named args numbering

4874602

Co-authored-by: Rylan Polster <rslpolster@gmail.com>

Rylan12 approved these changes Aug 3, 2022

View reviewed changes

issyl0 mentioned this pull request Aug 3, 2022

official_taps: Add cask-fonts, cask-drivers taps #13640

Merged

7 tasks

dev-cmd/contributions: Friendlier --repositories help text

11d661c

issyl0 merged commit 154806d into Homebrew:master Aug 3, 2022

issyl0 deleted the cmd-contributions branch August 3, 2022 18:05

issyl0 mentioned this pull request Aug 3, 2022

Enhance brew contributions. #13642

Closed

issyl0 mentioned this pull request Aug 9, 2022

cli/parser: Allow comma_array to take multiple names #13672

Closed

7 tasks

github-actions bot added the outdated PR was locked due to age label Sep 4, 2022

github-actions bot locked as resolved and limited conversation to collaborators Sep 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

dev-cmd/contributions: Retrieve a user's repo contributions over time #13603

dev-cmd/contributions: Retrieve a user's repo contributions over time #13603

issyl0 commented Jul 24, 2022

BrewTestBot commented Jul 24, 2022

Rylan12 left a comment

issyl0 commented Jul 24, 2022

Rylan12 commented Jul 24, 2022

issyl0 commented Jul 24, 2022

MikeMcQuaid commented Jul 25, 2022

MikeMcQuaid commented Jul 25, 2022

MikeMcQuaid left a comment

carlocab commented Jul 25, 2022

BrewTestBot commented Jul 26, 2022

issyl0 commented Jul 26, 2022

issyl0 commented Jul 28, 2022

MikeMcQuaid left a comment

MikeMcQuaid Jul 29, 2022

issyl0 Jul 29, 2022

issyl0 commented Jul 31, 2022

issyl0 commented Aug 3, 2022

Rylan12 left a comment

Rylan12 Aug 3, 2022

issyl0 Aug 3, 2022

Rylan12 Aug 3, 2022

issyl0 Aug 3, 2022

Rylan12 left a comment

issyl0 commented Aug 3, 2022

MikeMcQuaid commented Aug 4, 2022

	comma_array "--repos=",
	comma_array "--repositories=", "--repos",

	"Supported repositories: #{SUPPORTED_REPOS.join(", ")}. " \
	"Supported repositories: #{SUPPORTED_REPOS.map { \|t\| "`#{t}`" }.to_sentence}." \

dev-cmd/contributions: Retrieve a user's repo contributions over time #13603

dev-cmd/contributions: Retrieve a user's repo contributions over time #13603

Conversation

issyl0 commented Jul 24, 2022

BrewTestBot commented Jul 24, 2022

Rylan12 left a comment

Choose a reason for hiding this comment

issyl0 commented Jul 24, 2022

Rylan12 commented Jul 24, 2022

issyl0 commented Jul 24, 2022

MikeMcQuaid commented Jul 25, 2022

MikeMcQuaid commented Jul 25, 2022

MikeMcQuaid left a comment

Choose a reason for hiding this comment

carlocab commented Jul 25, 2022

BrewTestBot commented Jul 26, 2022

issyl0 commented Jul 26, 2022

issyl0 commented Jul 28, 2022

MikeMcQuaid left a comment

Choose a reason for hiding this comment

MikeMcQuaid Jul 29, 2022

Choose a reason for hiding this comment

issyl0 Jul 29, 2022

Choose a reason for hiding this comment

issyl0 commented Jul 31, 2022

issyl0 commented Aug 3, 2022

Rylan12 left a comment

Choose a reason for hiding this comment

Rylan12 Aug 3, 2022

Choose a reason for hiding this comment

issyl0 Aug 3, 2022

Choose a reason for hiding this comment

Rylan12 Aug 3, 2022

Choose a reason for hiding this comment

issyl0 Aug 3, 2022

Choose a reason for hiding this comment

Rylan12 left a comment

Choose a reason for hiding this comment

issyl0 commented Aug 3, 2022

MikeMcQuaid commented Aug 4, 2022