Commit queue overwrites commit author #52967

Activity

1. avivkeller

   added

   confirmed-bugIssues and PRs for confirmed bugs.Issues and PRs for confirmed bugs.

   metaIssues and PRs related to the general management of the project.Issues and PRs related to the general management of the project.

   on May 13, 2024
2. avivkeller commented on May 13, 2024

   

   avivkeller

   on May 13, 2024

   Member

   More actions

   I haven't looked at the code for the bot, but theoretically could we just replace the PR author setup with the first commit author?

   I'm happy to take a look at it, but I'm not a CODEOWNER, just a triage member

   @nodejs/github-bot
3. aduh95 commented on May 14, 2024

   

   aduh95

   on May 14, 2024

   Last edited by aduh95

   Contributor

   More actions

   I think that's a limitation of the GitHub API – the alternatives/workaround are:

   • land the PR with commit-queue-rebase PRs the Commit Queue should land as multiple self-contained commits. (i.e. no purple merge)
   • land the PR manually
   • add the original author as a co-author

   I've been trying to do the latter when I notice the commit author is not the same person that sent the PR. I guess we could implement a check to refuse to land a PR when the author will be wrong.
4. avivkeller commented on May 14, 2024

   

   avivkeller

   on May 14, 2024

   Member

   More actions

   I guess we could implement a check to refuse to land a PR when the author will be wrong.

   That seems (IMO) a good solution, but I'm far from the person who would be affected by any of this
5. marco-ippolito commented on May 15, 2024

   

   marco-ippolito

   on May 15, 2024

   Last edited by marco-ippolito

   Member

   More actions

   This changed recently, before when you added the "commit-queue-squash" it would squash on the first commit and keep the author of the first commit as the author of the landed commit.
   Now the author of the pr becomes the author of the landed commit regardless if the first commit is theirs
6. targos commented on May 15, 2024

   

   targos

   on May 15, 2024

   MemberAuthor

   More actions

   I made a little experiment using the GH UI: targos#18, targos@adf6657
   Squash and merge indeed changes the author (it doesn't let you keep it, you can only select the email address you want to be associated with the commit).
   It adds a Co-authored-by: field to the commit message, though.
7. aduh95 commented on May 15, 2024

   

   aduh95

   on May 15, 2024

   Contributor

   More actions

   This changed recently, before when you added the "commit-queue-squash" it would squash on the first commit and keep the author of the first commit as the author of the landed commit.

   I think you're misremembering, AFAIK it's always been the case that GH uses the "PR opener" as commit author – and we're using the GH API since #40666.
8. marco-ippolito commented on May 15, 2024

   

   marco-ippolito

   on May 15, 2024

   Member

   More actions

   This changed recently, before when you added the "commit-queue-squash" it would squash on the first commit and keep the author of the first commit as the author of the landed commit.

   I think you're misremembering, AFAIK it's always been the case that GH uses the "PR opener" as commit author – and we're using the GH API since #40666.

   I remember #45597 maybe it was landed with git node land