GitHub project contribution badges

What is the difference between the GitHub project badges “Committer” and “Contributor”?

For Committer the description says “created a commit to https://github.com/apache/kafka

For Contributor the description says “contributed an accepted pull request to https://github.com/apache/kafka

Since I am not an Apache committer, I’ve never directly committed to Apache Kafka. All my contributions have been accepted pull requests. Nevertheless I have the Committer badge here. So I am wondering why and what the difference between “created a commit” and “contributed an accepted pull request” is. Similar applies to the other badges that contain the word “Committer” and “Contributor”.

Additionally, it seems that the definition of Committer and Contributor here and in Apache Kafka differ, which might be confusing.

Yeah, I’m trying to figure this out @Bruno - I’d been discussing it with @mjsax too.

We’ve got the https://github.com/discourse/discourse-github plugin setup, which pulls in contribution and commits from https://github.com/apache/kafka/.

Looking at https://github.com/apache/kafka/graphs/contributors it shows you’ve got commits:

Do you know what the terminology mismatch is here? We can retitle (or even disable) the badges as needed once we’ve figured it out.

1 Like

Thank you for looking into this @rmoff,

The commits that you see in the screenshot, all originate from accepted pull requests.

My question is what is the difference between
“created a commit to https://github.com/apache/kafka”
and
“contributed an accepted pull request to https://github.com/apache/kafka”

I’m going to disable the Committer badges, because (a) it’s ambiguous in exactly the way you say, and (b) Committer is an overloaded term in the context of ASF, and if the plugin it using it in a different way then we’re going to confuse people

/cc @mjsax

1 Like

So we now have these three github badges:

I’ve just updated the thresholds, in theory we should see some users get Gold, I don’t know how often the refresh happens.

For ASF committers there is the @committers discourse group which is separate from the badges, and maintained manually.

1 Like

Thanks Robin!

This makes sense to me. We should follow ASF terminology and use the
term “committer” for well, committers only. And we have the
corresponding group for those people.

Everybody else should be a “contributor” and the names of the three
contributor badges make sense to me!

2 Likes

Yes, removing “Committers” makes also sense to me.

Now, I am wondering why I am not an Amazing Contributor since I am amazing :grin: and I have more than 50 accepted pull requests.

I think the update is async – I also did not get the gold badge right away… Seems I am the only one who is amazing (for now) :smiling_imp:

I found this post which says that the refresh is every four hours.

I’ve logged a support ticket since this plugin seems kinda black-box (although the source code is here if anyone wants to take a look).

I’m wondering if the config that I set (github gold badge min commits ) applies only to commits and not contributions - this might make sense if the default threshold is 250 and @Bruno has 87 (< 250) and @mjsax 385 (>250)

The code’s here

I’m not a ruby coder, but glancing at it it seems clear that there are some hardcoded thresholds being used as well as the configurable ones.

Let’s see what Discourse support come back with.

Discourse support have come back with some info:

Bruno has made 87 commits to the repo, but only 42 of those commits have been made with the ‘contributor’ role. His other commits were made with the ‘committer’ role

Looking at the Kafka repo for @Bruno’s commits I think this is the difference that the badge plugin is making:

So whilst Github counts all of this as commits:
image

the plugin splits the difference between contributions (committed by someone else) and commits (committed directly).

What I’m not clear about is how someone who is not a Committer (in the ASF sense) on the Apache Kafka project can commit directly to the repo - @mjsax can you explain?

1 Like

I think the best would be to have a badge for the sum of contributions and commits because it would be the least confusing criteria to grant the badge. Don’t you think?

I am not 100% sure, but only committers can merge PRs.

It seems that there is a cut-off point and older PR are labeled as “authored” while newer ones are labeled “committed”, thus, it might be a change how we merge PRs? But I am not sure what changed…

The only thing I could think of (but this changes happened earlier) is as follows: In early days, we could not merge directly on GitHub, because GitHub is not the primary repository but a mirror of a ASF hosted git; we still did PRs and reviews over GitHub, but merged those PRs into the primary ASF repos and the commit was mirrored into GitHub afterwards. – Later on, ASF allows us to merge directly on GitHub.

:man_shrugging:

I agree with Bruno though that it make not sense to no count all. It’s an artificial distinction.

I’ve been poking around this some more.

The root issue seems to be that the plugin is picking up some contributions (merged PRs) as commits. This means that the numbers are off, and users who do not have commit rights on the repo are incorrectly granted committer badges.

Here’re some examples, both PRs from @Bruno - who does not have commit rights on the repo :

  • 20 Dec 2019 1d21cf166a8ed7def514a1860318e74b64dd47da merged by mjsax. Plugin categories this as a contribution (correctly).
  • 29 Oct 2020 a85b9440118ae5aa442d2e1e70b379841f941ddc merged by ableegoldman. Plugin incorrectly categories this as a commit (incorrectly).

The role field is how it is viewing the commit vs contribution (code):

SELECT * from github_commits
where sha IN ('a85b9440118ae5aa442d2e1e70b379841f941ddc','1d21cf166a8ed7def514a1860318e74b64dd47da')

This looks like the code that decides whether it’s a PR (contribution) or not:

Looking at the data the GitHub API returns we can see the values for the two commits above:

$ curl --show-error --silent -H "Accept: application/vnd.github.v3+json" \
  https://api.github.com/repos/apache/kafka/commits/1d21cf166a8ed7def514a1860318e74b64dd47da | jq '.commit.author,.commit.committer'
{
  "name": "Bruno Cadonna",
  "email": "bruno@confluent.io",
  "date": "2019-12-20T22:21:12Z"
}
{
  "name": "Matthias J. Sax",
  "email": "matthias@confluent.io",
  "date": "2019-12-20T22:21:12Z"
}

$ curl --show-error --silent -H "Accept: application/vnd.github.v3+json" \
  https://api.github.com/repos/apache/kafka/commits/a85b9440118ae5aa442d2e1e70b379841f941ddc | jq '.commit.author,.commit.committer'
{
  "name": "Bruno Cadonna",
  "email": "bruno@confluent.io",
  "date": "2020-10-29T23:10:30Z"
}
{
  "name": "GitHub",
  "email": "noreply@github.com",
  "date": "2020-10-29T23:10:30Z"
}

So the second commit (a85b9440118ae5aa442d2e1e70b379841f941ddc) despite still being a PR that was merged, has fallen victim to the logic in the plugin code shown above, which says that if the committer name is GitHub then it must be a direct commit by the author - and I don’t think that’s a valid assumption for it to make.

3 Likes

Do you think we could get the plugin changed? Or do we need to live with it as-is?

I’ve been emailing Discourse about this in parallel, and instead of duplicating comms have just invited @simon.cossar from Discourse to this thread :slight_smile:

@simon.cossar it would be great to get feedback on :

  1. Is the plugin faulty (as I believe it is, per the above) , since it is incorrectly counting some contributions as commits?
  2. Can we get the plugin fixed to act how we believe it should?
  3. Failing that, can the plugin be enhanced, for example add a boolean config to count commits under contributions (which would fix our issue, I believe)

thanks, Robin.

2 Likes

Thanks for looking into this. I’ll create a topic about the issue on our internal forum and see if our developers agree that the logic is faulty. If it is, we should be able to fix the issue.

Since it’s Friday today, I’m unlikely to be able to get back to you about this until early next week.

I’ll check with the developers about this as well.

2 Likes

This has now been fixed, and deployed to our site :tada:

@Bruno you now have all your well-deserved badges :slight_smile: image

2 Likes

@rmoff Thanks a lot for following up on that!

@simon.cossar Thanks a lot for the support!

I think we have a badge now that is easily understandable by users of this forum.

@mjsax Finally, I am also amazing! :grin:

Best,
Bruno

2 Likes