commit history attributes commit to committer instead of author

[simonjayhawkins]

https://github.com/pandas-dev/pandas/commits/master since 690e382

[simonjayhawkins]

@pandas-dev/pandas-core I think we need to not merge until this is resolved. Any ideas?

[datapythonista]

Reported it to GitHub, will update with any info from their side (their issues are paradoxically not GitHub issues, and not public).

[wesm]

Wow that's seriously buggy

[datapythonista]

Just received this answer from GitHub:

Hi Marc,

Thanks for writing in about this!

This is an intentional change that our team recently deployed and I'm sorry to hear that's causing trouble.

When a user chooses to squash and merge a PR they are taking authorship of the commit. If they were merging the PR and creating a merge commit that merge commit would be attributed to them as author, so we wanted to be consistent with that hence the change. We are also aware that we should be setting the author as a co-author and we can see that's not happening every time.

We've passed this on to our team both as feedback about the recent changes and to look into why co-authors are not being set as expected.

Please let me know if you have any further questions!

Cheers,
Adrienn
GitHub Support

[wesm]

I think this is a really bad change. In Arrow we include explicit by-lines in our commit messages to prevent this kind of shenanigans

apache/arrow@b4acb0b

[datapythonista]

Was in the phone with someone from GitHub, and looks like they are thinking how they can add two authors to the PR. But doesn't seem like anything is going to change soon.

I think we've got the next alternatives:

1. Just continue as normal and get the authors wrong from now on
2. Use the author headers @wesm mentions (may be can be added automatically from GitHub actions)
3. Go back to manually rebasing. If I understood correctly, the reason is that we're squashing the commits automatically, if users squash commits manually and we stop using the squashing option, then they'll keep the authorship

I can have a look at automating 2 if people things this is a priority, I guess it's doable, but doesn't seem easy.

[rth]

See additional response in scikit-learn/scikit-learn#16640 (comment).

Basically there is a change in behavior but that should be somewhat offset by the automatic addition of a Co-authored-by tag which is currently sometimes not working due to a bug.

[wesm]

@datapythonista you can also use the CLI PR merge tool (the one we have in https://github.com/apache/arrow/blob/master/dev/merge_arrow_pr.py would be easy to adapt)

[datapythonista]

Looks like GitHub in reconsidering the change:

https://twitter.com/natfriedman/status/1235613840659767298

Probably worth waiting before merging PRs

[rlenferink]

It seems that k8s-ci-robot did some nice contributions today 😄: https://github.com/kubernetes/website/commits/066b9f3b8c4800abc35c4d08ec3b06129a9f53c7

[jbrockmendel]

In the interim, can we manually fix the attribution when doing the squash+merge?

[datapythonista]

Been contacted by github that this has already been reverted, closing.