eclipse-platform / .github

Common contribution content for eclipse-platform repositories
https://www.eclipse.org/eclipse/
4 stars 10 forks source link

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

Open jukzi opened 2 weeks ago

jukzi commented 2 weeks ago

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 commented 2 weeks ago

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

netomi commented 2 weeks ago

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 <jjohnstn@redhat.com>
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 commented 2 weeks ago

no, i did it all alone.

netomi commented 2 weeks ago

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 commented 2 weeks ago

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": "jjohnstn@redhat.com",
      "date": "2024-06-12T18:06:59Z"
    },
    "committer": {
      "name": "Jeff Johnston",
      "email": "jjohnstn@redhat.com",
      "date": "2024-06-12T18:06:59Z"
    },

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

netomi commented 2 weeks ago

I was testing also with git itself:

commit 5c73ece032b339b0f3960916a861bb92230be2ef (HEAD -> master, origin/master, origin/HEAD)
Author:     Jeff Johnston <jjohnstn@redhat.com>
AuthorDate: Tue Jun 18 16:24:46 2024 +0200
Commit:     Jörg Kubitz <51790620+jukzi@users.noreply.github.com>
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 <jjohnstn@redhat.com>
AuthorDate: Mon Jun 17 10:05:50 2024 -0400
Commit:     GitHub <noreply@github.com>
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 <jjohnstn@redhat.com>
AuthorDate: Fri Jun 14 11:50:38 2024 -0400
Commit:     GitHub <noreply@github.com>
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 <jjohnstn@redhat.com>
AuthorDate: Tue Jun 11 17:09:57 2024 -0400
Commit:     GitHub <noreply@github.com>
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 commented 2 weeks ago

Could be related to Codespaces:

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

jukzi commented 2 weeks ago

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

jukzi commented 2 weeks ago

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 commented 2 weeks ago

There are 2 cases that are distinct imho:

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 11 hours ago

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.