Skip to content
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

Wrong "Author" / "Committer" on GitHub PR merges #215

Open
jukzi opened this issue Jun 19, 2024 · 13 comments
Open

Wrong "Author" / "Committer" on GitHub PR merges #215

jukzi opened this issue Jun 19, 2024 · 13 comments
Labels
bug Something isn't working

Comments

@jukzi
Copy link
Contributor

jukzi commented Jun 19, 2024

Sometime wrong names are shown in the history

For example in jdt.ui
image

image
image
https://github.com/eclipse-jdt/eclipse.jdt.ui/pull/1466/commits

  1. JJohnstn was never involved in this particular PR. It should not be possible that i create PRs in the name of someone else. I do not know why it happened.
  2. whenever JJohnstn merges then GitHub is shown as Committer

@eclipse-jdt/eclipsefdn-security

@jukzi jukzi added the bug Something isn't working label Jun 19, 2024
@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

@netomi @mbarbero it seems that tagging "@eclipse-jdt/eclipsefdn-security" is not working here

@netomi
Copy link

netomi commented Jun 19, 2024

looking at the commit in question at https://github.com/eclipse-jdt/eclipse.jdt.ui/commit/f375359a1eefd6479d79fda05e2e4e0e5bd8a1cc.patch

shows that

From f375359a1eefd6479d79fda05e2e4e0e5bd8a1cc Mon Sep 17 00:00:00 2001
From: Jeff Johnston <[email protected]>
Date: Tue, 18 Jun 2024 16:24:46 +0200
Subject: [PATCH] AbstractAnnotateAssistTests: more information on NPE #736

so I dont know how that PR was created, but from the data it looks consistent. Did you receive a patch from Jeff Johnston or cherry picked this commit from his fork in your own fork?

@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

no, i did it all alone.

@netomi
Copy link

netomi commented Jun 19, 2024

if that is the case, maybe your local setup of user.email is mixed up, otherwise I have no idea how this can happen.

@netomi
Copy link

netomi commented Jun 19, 2024

For the other cases where Jeff Johnston is shown as author and GitHub as committer, I did take a look at the raw data from the GitHub API and it looks fine:

tn@proteus:~/workspace/eclipse/EclipseFdn/otterdog-configs$ gh api   -H "Accept: application/vnd.github+json"   -H "X-GitHub-Api-Version: 2022-11-28"   /repos/eclipse-jdt/eclipse.jdt.ui/commits/9ecc09c51922e9fd89c0a80495ba487e327b5bf6
{
  "sha": "9ecc09c51922e9fd89c0a80495ba487e327b5bf6",
  "node_id": "C_kwDOHJK_uNoAKDllY2MwOWM1MTkyMmU5ZmQ4OWMwYTgwNDk1YmE0ODdlMzI3YjViZjY",
  "commit": {
    "author": {
      "name": "Jeff Johnston",
      "email": "[email protected]",
      "date": "2024-06-12T18:06:59Z"
    },
    "committer": {
      "name": "Jeff Johnston",
      "email": "[email protected]",
      "date": "2024-06-12T18:06:59Z"
    },

not sure what tool you use to show the commit history, but maybe it is not accurate.

@netomi
Copy link

netomi commented Jun 19, 2024

I was testing also with git itself:

commit 5c73ece032b339b0f3960916a861bb92230be2ef (HEAD -> master, origin/master, origin/HEAD)
Author:     Jeff Johnston <[email protected]>
AuthorDate: Tue Jun 18 16:24:46 2024 +0200
Commit:     Jörg Kubitz <[email protected]>
CommitDate: Wed Jun 19 07:51:59 2024 +0200

    AbstractAnnotateAssistTests: more information on NPE #736
    
    https://github.com/eclipse-jdt/eclipse.jdt.ui/issues/736

commit 76cb0af9adf70b36792bf3d2fb2e7cc23f119fe3 (tag: I20240618-1800, tag: I20240617-1800)
Author:     Jeff Johnston <[email protected]>
AuthorDate: Mon Jun 17 10:05:50 2024 -0400
Commit:     GitHub <[email protected]>
CommitDate: Mon Jun 17 10:05:50 2024 -0400

    Fix move of record to new file refactoring when component accessed (#1453)
    
    - add check in MemberVisibilityAdjustor.rewriteVisibility() to ignore
      record fields
    - add new test to MoveInnerToNewTests16
    - fixes #1419

commit d2721bd6e858f6b55a4b66733b0a039e144df49e (tag: I20240616-1800, tag: I20240615-1800, tag: I20240614-1800)
Author:     Jeff Johnston <[email protected]>
AuthorDate: Fri Jun 14 11:50:38 2024 -0400
Commit:     GitHub <[email protected]>
CommitDate: Fri Jun 14 11:50:38 2024 -0400

    Fix NPE in OverriddenAssignmentFixCore moveDown method (#1457)
    
    - fixes #1454

commit 68a3ac2f9d824457bb3c72b19ae7df188fe3a003 (tag: I20240613-1800, tag: I20240612-1800, tag: I20240612-0510, tag: I20240611-1800)
Author:     Jeff Johnston <[email protected]>
AuthorDate: Tue Jun 11 17:09:57 2024 -0400
Commit:     GitHub <[email protected]>
CommitDate: Tue Jun 11 17:09:57 2024 -0400

so it looks like there is some discrepancy in this case which I cant explain right now. Maybe he did this changes with the Web UI? I can search a bit about this issue.

@netomi
Copy link

netomi commented Jun 19, 2024

Could be related to Codespaces:

https://github.com/orgs/community/discussions/45065

@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

I am using Eclipe / egit. That shows GitHub:
image

@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

If i can upload a patch with a other users "Author" and Github overrides "Committer" at the end both fields would be wrong - totally blind to the real committer.

@netomi
Copy link

netomi commented Jun 19, 2024

There are 2 cases that are distinct imho:

  • committer being displayed as GitHub [email protected]
  • author shown as somebody that allegedly has not been involved in the commit

for the first case, evidence is strong that it is related to the use of Codespaces. The behavior is exactly as described in the referenced discussion, please check with John if he did so.

for the second case I have no idea, you have deleted the branch in your fork already, maybe you can restore it for further inspection?

@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

for the second case I have no idea, you have deleted the branch in your fork already, maybe you can restore it for further inspection?

already deleted :-(

@jukzi
Copy link
Contributor Author

jukzi commented Jun 19, 2024

for the first case, evidence is strong that it is related to the use of Codespaces. The behavior is exactly as described in the referenced discussion, please check with John if he did so.

@jjohnstn which workflow do you use to merge PRs - such that your name is not shown as Committer?

@HannesWell
Copy link
Member

for the first case, evidence is strong that it is related to the use of Codespaces. The behavior is exactly as described in the referenced discussion, please check with John if he did so.

@jjohnstn which workflow do you use to merge PRs - such that your name is not shown as Committer?

From my experience this happens if one uses squash-and-merge on PRs. There might be other cases, tough.
That's why I always use Rebase and Merge. This sets the committer correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants