Skip to content

commit history attributes commit to committer instead of author #32457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
simonjayhawkins opened this issue Mar 5, 2020 · 12 comments
Closed

Comments

@simonjayhawkins
Copy link
Member

simonjayhawkins commented Mar 5, 2020

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

@simonjayhawkins
Copy link
Member Author

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

@datapythonista
Copy link
Member

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

@wesm
Copy link
Member

wesm commented Mar 5, 2020

Wow that's seriously buggy

@datapythonista
Copy link
Member

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
Copy link
Member

wesm commented Mar 5, 2020

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
Copy link
Member

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
Copy link
Contributor

rth commented Mar 5, 2020

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
Copy link
Member

wesm commented Mar 5, 2020

@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
Copy link
Member

Looks like GitHub in reconsidering the change:

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

Probably worth waiting before merging PRs

@rlenferink
Copy link

rlenferink commented Mar 5, 2020

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

@jbrockmendel
Copy link
Member

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

@datapythonista
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants