GitHub project contribution badges

Bruno · 1 December 2020 12:30

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

For Committer the description says “created a commit to GitHub - apache/kafka: Mirror of Apache Kafka”

For Contributor the description says “contributed an accepted pull request to GitHub - apache/kafka: Mirror of 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.

rmoff · 1 December 2020 15:23

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

We’ve got the GitHub - discourse/discourse-github plugin setup, which pulls in contribution and commits from GitHub - apache/kafka: Mirror of Apache Kafka.

Looking at Contributors to apache/kafka · GitHub 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.

Bruno · 1 December 2020 15:55

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”

rmoff · 1 December 2020 16:28

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

rmoff · 1 December 2020 16:29

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.

mjsax · 1 December 2020 17:14

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!

Bruno · 1 December 2020 18:18

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

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

mjsax · 1 December 2020 20:11

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)

rmoff · 2 December 2020 10:01

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)

rmoff · 2 December 2020 10:06

The code’s here

github.com

discourse/discourse-github/blob/3ca98d36fb4756ad41848cd35c9d5703dc59a0d1/app/lib/github_badges.rb#L84-L112


      
          def self.grant_committer_badges!
            emails = GithubCommit.where(
              merge_commit: false,
              role_id: CommitsPopulator::ROLES[:committer]
            ).pluck(:email)
          
          
  bronze, silver, gold = committer_badges
          
          
  granter = GithubBadges::Granter.new(emails)
            granter.add_badge(bronze, as_title: false, threshold: 1)
            granter.add_badge(silver, as_title: true, threshold: 25)
            granter.add_badge(gold, as_title: true, threshold: 1000)
            granter.grant!
          end
          
          
def self.grant_contributor_badges!
            emails = GithubCommit.where(
              merge_commit: false,
              role_id: CommitsPopulator::ROLES[:contributor]
            ).pluck(:email)

This file has been truncated. show original

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.

rmoff · 3 December 2020 09:48

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:

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?

Bruno · 3 December 2020 10:12

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?

mjsax · 3 December 2020 16:53

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.

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

rmoff · 3 December 2020 17:55

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:

github.com

discourse/discourse-github/blob/3ca98d36fb4756ad41848cd35c9d5703dc59a0d1/app/lib/commits_populator.rb#L74-L79


      
          def is_pr?(commit)
            commit.commit.author.name != commit.commit.committer.name && commit.commit.committer.name != "GitHub"
            # GitHub has a special account that acts as the committer of commits that are
            # created via the UI. For example when you merge your own PR (but not someone else's)
            # If this account is the committer of a commit then don't count it as a PR
          end

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.

mjsax · 4 December 2020 09:33

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

rmoff · 4 December 2020 09:56

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

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

Is the plugin faulty (as I believe it is, per the above) , since it is incorrectly counting some contributions as commits?
Can we get the plugin fixed to act how we believe it should?
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.

rmoff · 11 December 2020 07:11

This has now been fixed, and deployed to our site

github.com/discourse/discourse-github

FIX: Improve contributors detection

discourse:master ← discourse:fix-contributor-badge

opened 10:07PM - 09 Dec 20 UTC

OsamaSayegh

+157 -45

This plugin ships with 2 types of badges: Contributor and Committer. Currently,… these badges are granted by replicating the git history of the configured repositories in the database using Github's REST API, and then looking up users by the emails on the commits and hand out badges. A commit is considered a 'contribution' if it comes from a pull request that was not created and merged by the same user. The commits endpoint of Github's REST API doesn't include whether a commit comes from a PR or not. So, our only way to detect contributions is to compare the `committer` and `author` fields of the commit object that we get from the API. This method worked fairly well up until December 2019 when Github changed something that made the `committer` always be set to `GitHub` and not the user who merged the pull request. So, this commit replaces the REST API with GraphQL which allows us to include pull request data (if any) with each commit object. This gives us a reliable way for detecting contributions.

@Bruno you now have all your well-deserved badges

Bruno · 11 December 2020 09:26

@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!

Best,
Bruno

Topic		Replies	Views
Developer’s Guide to Contributing Code to Kafka [Kafka Summit 2022] Summit	0	3210	24 April 2022
Hi, I'm Florian from France! Lounge	2	3093	3 March 2021
🎧 Apache Kafka 3.5 - Kafka Core, Connect, Streams, & Client Updates News and Blogs	0	1489	15 June 2023
Worth the time? An Open Source Kafka Dashboard / GUI Graphical tools	0	3045	21 June 2022
🎧 Apache Kafka 3.1 - Overview of Latest Features, Updates, and KIPs News and Blogs	0	2608	24 January 2022

GitHub project contribution badges

Related Topics