Contributors of squashed commits don't get any love

[patrickleet]

I contributed a few days of work to an open source project. It was merged into a sub-branch, that was then squashed and merged by the library's original author to keep a clean git history, erasing my status as a contributor.

Now when I send people the library and tell them I'm a contributor, they don't believe me, because I've been reduced to a footnote in a commit message.

:(

Maybe when a squash and merge is done through the UI at least, the information about who contributed those changes could be tracked.

[aspiers]

See also #1368 which is more or less the same thing, but gives more technical details.

[jmarshall]

(This is a different problem from #1368.)

GitHub interprets Co-authored-by commit message trailers to represent additional authors on commits. So the infrastructure is in place and the Squash merge button could add such trailers to the resulting commit message so that all contributors got their attributions — though someone would still have to select one as a distinguished main author of the commit.

[negebauer]

It would be very cool if Github added Co-authored-by on pr merge!

[mcking65]

Is there any way for a maintainer to work around this? I maintain protected branches in a repo where I'm not an admin. For most contribbutions, we squash and merge into the protected branches.

We let members of our team create branches and then create PRs, so they will still get credit when I squash and merge from one of their PRs. However, if someone forks and submits a PR with the compare branch in their fork, I cannot merge into the protected branch because the required Travis statuses are not reported for the branch in their fork. So, I have to merge their PR into a new unprotected feature branch and then create a new PR with that branch as the compare branch. At this point, if I create a PR and then squash that person's commits, all their author info disappears and they get 0 credit.

Is there any possible way for me to manually work around this, e.g., something I can add to the commit statement that would cause the author's contributions to be counted?

[jmarshall]

At this point, if I create a PR and then squash that person's commits, all their author info disappears and they get 0 credit.

How are you managing to do that?

If the original commits are

Author: Alice <alice@example.org>
Author: Alice <alice@example.org>
Author: Alice <alice@example.org>

and you squash these three with rebase -i or even with GitHub's squash and merge button (but cf #1368) you'll end up merging a single commit that looks like

Author: Alice <alice@example.org>
Committer: Matt <mcking65@example.org>

(when viewed with log --format=fuller) and GitHub will credit this to Alice, as desired.

This issue is that given these original three commits:

Author: Alice <alice@example.org>
Author: Alice <alice@example.org>
Author: Bob <bob@example.org>

it is non-trivial for Bob to get any credit. However, as noted previously, your work-around is to edit in a trailer to the squashed commit:

Author: Alice <alice@example.org>
Committer: Matt <mcking65@example.org>

Blah blah summarised commit messages.

Co-authored-by: Bob <bob@example.org>

and GitHub will credit this to both Alice and Bob.

[tibdex]

It would be very cool if Github added Co-authored-by on pr merge!

The Autosquash GitHub Action does that 😉.

[greysteil]

Fixed by this change. Thanks for everyone's input here, and props to @laserlemon for the fix.

[papb]

Nice, I guess this can be closed then?

[perk11]

I just had a variation this issue reappear. As I merged 4 Pull Requests using "Squash" in Github UI all of them got attributed to me, even though I didn't have any commits in them. Had to force push to master to fix this.

Edit: Yes, I meant I didn't have any commits in the Pull Request, thank you @papb

[papb]

@perk11

[...] I did have [...]

Did you mean didn't?

[greysteil]

We made a change yesterday that will be causing that. When you squash and merge a commit it will add the authors of the original commits as co-authors, but makes the merger the author of the commit. We did this for a couple of reasons: 1) Consistency with merge commits (where the creator of the merge commit is the user who clicks merge) 2) When clicking "squash and merge" you combine several commits, and have the option to rewrite the commit message. By doing so, you have the last word in "authoring" the resulting squashed commit. Under the old setup (PR creator credited as first author) we had a few people get in touch concerned about being credited as the "author" on commits where the commit message had been changed.

We review all changes like this, so are very happy to hear feedback.

[perk11]

Thank you for the response. To me this just means I'll be back to CLI when I need to squash commits.

Co-contributors don't show up on Github or in git in a visible way.

And in my scenario 95% of the time I just review the code and maybe make some changes to the commit message, this is a very small amount of work compared to what the Pull Request author does and yet I get attributed as the main contributor to the commit?..

And when I merge it from Github, after I look at commit log at Github, git log, git blame, etc it will be just my name everywhere... This is not useful for practical reasons when an author of change needs to be identified and also is denying the actual commit authors the attribution.

It makes more sense for the merge commit because merge commit keeps the original commits intact, but with squash you only get the end result.

[tpoterba]

We made a change yesterday that will be causing that. When you squash and merge a commit it will add the authors of the original commits as co-authors, but makes the merger the author of the commit.

I very much understand the need to fix authorship when there are multiple authors of squashed commits, but if there is only one author (this is extremely common in pull requests made by our team), shouldn't that be the author of the merged commit, not the person (or bot) who pressed the merge button?

[greysteil]

It looks like the change we've made has a bug in it which means the PR author isn't getting a co-author credit on the squashed commit. We're working on a fix for that now, and I think that's the cause of a large part of the problem here (and explains why you're not seeing the co-author show up in GitHub @perk11 - co-authors do get displayed, as shown on the commits here for example).

@tpoterba - PRs with a single author are an interesting case. We originally considered doing the above in the case where the commit message is not changed during merging. Doing so would introduce an inconsistency with other squash and merge commits, though, so we wanted to ship and learn here. It's also worth noting that the command line equivalent, git merge --squash, makes the merger the author of the commit.

We're working to get the co-author bug fixed now, and will review after.

[sdrioux]

This is also an issue for our team. We have QA do the actual squashing and merging of PRs into our base branch - however, we would like to be able to track which developer was the author of the commits in the PR. Otherwise thing like git blame become useless. Similar to @tpoterba, 99% of the time these commits have been authored by a single developer.

[chrislarson]

Either the requirements change or the bug (not sure which) is negatively impacting our code base in a big way.

Git Blame is all wrong after merges the past two days. If I have a question about a line of code, I should go to the person who authored the code not the person who authored the commit message.

[greysteil]

Thanks for all the feedback here, and to folks who emailed GitHub support, too.

We're going to revert the default behaviour back to the PR author (that is, the creator of the PR) being used as the author for the squashed git commit. The revert will go out in the next 24 hours. We'll have a rethink on how we solve the problems that the initial change was intended to fix.

[HyukjinKwon]

@greysteil, just to clarify, which previous behaviour are we going back? A. The PR author alone (as PR creator) B. The PR author as PR creator, and the committer as the PR merger

Also do you guys plan to eventually fix as below? C. The PR merger as the main author, and PR creator as co-author

I noticed Github changed the behaviour from B to A. A. hides the PR merger who likely reviewed and approved the PR, and is likely the maintainer of the project. B. looked most reasonable to me and consistent with manual git rebase, squash and push by the maintainer.

C. looks still odd. I think the PR merger should be as co-author who reviewed the change, and the PR creator should be the author who actually wrote the codes.

Also, was this behaviour made based solely upon the discussion made within Github teams? I think this case shows such behaviour changes should be made based upon actual user feedback before you guys make the changes. Commits logs are supposed to be permanent, and you guys made inconsistent commits log due to this behaviour change back-and-forth in multiple projects silently.

[greysteil]

@HyukjinKwon - we've just deployed a reversion of the behaviour change. The behaviour of squash-and-merge is now as follows:

• the PR author becomes the squashed commit's author
• all commit authors and co-authors are credited as co-authors on the squashed commit (using the co-authored-by trailer in the (editable) commit message)
• the merger is uncredited by default. If they would like credit they can add their name as a co-author to the commit message
• the squashed commit's "committer" is GitHub (as it is for merge commits), verified with our GPG key

This is a reversion to the behaviour we shipped for squash-and-merge back in December 2019, with a couple of additional fixes that ensure the co-authored-by trailers use the correct email addresses for all co-authors, and don't miss anyone.

Thanks for everyone's patience here as this was deployed, and apologies for my mistake switching the behaviour.

[HyukjinKwon]

Thanks for clarification, @greysteil.

One last question though,

the squashed commit's "committer" is GitHub (as it is for merge commits), verified with our GPG key

Is there any way to mimic the previous behaviour, committer as the merger, author as the PR creator, to include its merger in the commit log as anything such as a co-author or committer by default (or set once as a setting in a repository)? It is easier to find who to ask about some specific changes.

One problem by the behaviour you described is that it doesn't show who merged it by default. It matters for such case when maintainers in a project are too many and don't know each other, and difficult to track who merges which PR.

For example, Apache Spark has nearly ~100 committers from all different countries, companies and different timezones. The trackability is pretty crucial here.

[greysteil]

@HyukjinKwon not at the moment. We do add the PR's reference number to the default commit message for squash merges, so you can easily navigate back to the PR to determine who merged it in the UI. For more custom control over the author and committer of the squash commit, though, currently the only option is to merge the commits outside of GitHub.

[HyukjinKwon]

Sure, it adds a PR number. So it is not impossible to track. My point was that I will become inconvenient to navigate because it forces people to click and check each commit. In the project I mentioned above, I check the commit history very often every day for multiple purposes such as PR review or talking to another maintainer. Sometimes I skim only PR titles too, and talk with the maintainer.

In addition, it disables people to programmatically track who committed specifically by directly using git, e.g. https://stackoverflow.com/questions/9597410/list-all-developers-on-a-project-in-git and https://stackoverflow.com/questions/1265040/how-to-count-total-lines-changed-by-a-specific-author-in-a-git-repository. At least, it forces them to change their existing way.

It would be great if we have an alternative that doesn't hurt user experience when such behaivour changes happen next time.

[tanzislam]

@greysteil

The behaviour of squash-and-merge is now as follows:

• the PR author becomes the squashed commit's author

I think this should be changed to: the author of the first commit in the PR (not the person who raised the PR) becomes the squashed commit's author. That will credit the person who made the original commit, and not the (other) person who decided that it is now a good time to advertise it for merging.

[tanzislam]

Also:

• the merger is uncredited by default. If they would like credit they can add their name as a co-author to the commit message

I think auto-suggesting a "Merged By" (or similar) tag would be useful, and will partly address @HyukjinKwon's concern. The GitHub commit log UI should then take this tag into account when saying "committed by person X".

Note that setting the commiter to "GitHub" is the only way to have signed commits work with squash-merge, because any signatures in the original PR commits are lost in the squash-merge, and GitHub stores only its own GPG private key but not its users'. (The signature here is important to ascertain that GitHub's merge mechanism was indeed used, such that the result of the merge hasn't been tampered with to introduce unrelated changes.) However, for repo maintainers who don't care about commit signing at all, it may be worth having a repo-level option to disable commit signing on squash-merge, and use @HyukjinKwon's suggestion of using the merger as committer.

[greysteil]

@tanzislam - FYI, I've set up time with the PM responsible for pull requests and the merge button to talk through the context on this issue. He's on holiday today, but when he's back I'll make sure he has the right background to weigh in here. :octocat:

[Samk13]

Squashed commits are not showing on Github activity graph either, not sure if this is the same issue or should I open a new one for it. 🧐

[shehan-jay]

Is the number of lines added as co-author added here?

[webern]

It seems like the simplest fix would be to 'pick' the first commit and using 'fixup' for the subsequent commits.

[fneum]

Squashed commits are not showing on Github activity graph either, not sure if this is the same issue or should I open a new one for it.

Can confirm that we have the problem that while in squash & merge, all co-authors are associated with the commit, only one person is credited in the Github contributors view.