Hi Wilfred

I got your point now. It's all about who merged the PR.
I just checked some other OSS repos, looks like they are also using the
merge button, so with --format=fuller, it also shows to be committed by "
[email protected]".
Maybe that's OK as long as we have the author field correct?

On Wed, Jun 10, 2020 at 8:20 PM Wilfred Spiegelenburg <[email protected]>
wrote:

> Weiwei,
>
> The issue is not with the author in the commit it is with the committer.
> The GitHub user is assigned the commit. Github cannot (or will not) use the
> enduser that clicks the button as the committer.
> One of the reasons they gave is that the commits are signed. Github does
> not have the private key of the logged in user to sign the code when it
> commits.
> Sharing the private key is a security risk. If they would change to the end
> user clicking the button the commits cannot be signed.
>
> Checkout any of the repositories, run the following command:  git log
> --format=fuller
>
> This is the commit info from the commit you referenced:
>
> Author:     Kinga Marton <[email protected]>
> > AuthorDate: Wed Jun 10 20:40:34 2020 +0200
> > Commit:     *GitHub <[email protected] <[email protected]>>*
> > CommitDate: Wed Jun 10 11:40:34 2020 -0700
> >     [YUNIKORN-167] Fixed release tool after helm chart related changes
> (#7)
> >
> >     Updated the release script to replace the docker image version used
> in
> > the helm-chart with the correct release version. Only keep the latest
> > version helm charts in this repo.
>
>
> The commit is made by the user Github, not by Weiwei. The author is
> correctly picked up, the author is linked to the user that created the PR.
> Changing visibility on the email address only affects the author, it is not
> linked to the committer.
> As an example this is a merge for an older change of Kinga when she had her
> email address hidden. If you have hidden your email address on github it
> will look like this:
>
> Author:     Kinga Marton <[email protected]>
> > AuthorDate: Fri May 29 22:48:52 2020 +0200
> > Commit:     *GitHub <[email protected] <[email protected]>>*
> > CommitDate: Fri May 29 13:48:52 2020 -0700
> >     [YUNIKORN-180] Update user Guide (#161)
>
>
> Tracing back who did the commit in both cases is only possible by going
> through the github UI. Clicking on the PR. Scrolling to the point where the
> commit has left a merge comment.
>
> Wilfred
>
> On Tue, 2 Jun 2020 at 01:41, Sunil Govindan <[email protected]> wrote:
>
> > Thanks Wilfred
> > I agree.
> >
> > I think we can add these to our github and mandatory params for a PR
> >
> > Thanks
> > Sunil
> >
> > On Mon, Jun 1, 2020 at 9:07 PM Wilfred Spiegelenburg <
> [email protected]>
> > wrote:
> >
> > > I have had only one response to this discussion. I spoke offline to
> > Weiwei
> > > and looking at the lasts commits we are losing details and get badly
> > > formatted commit messages.
> > > * Committer is the generic github account. Here is an example of a
> commit
> > > from github.
> > > * Badly formatted messages as github does not insert line breaks.
> > >
> > > Since there was no strong -1 on this I am going to add the steps to the
> > > documentation, and ask everyone that commits to follow the simple
> manual
> > > merge steps:
> > >
> > > * git checkout master
> > >
> > > * git pull
> > >
> > > * git checkout -b <JIRA ID> master
> > >
> > > * git pull <FORK GIT>  <REMOTE PR branch>
> > >
> > > * git checkout master
> > >
> > > * git merge --squash <JIRA ID>
> > >
> > > * git commit --author “ORIGINAL AUTHOR <[email protected]>”
> > > * git push origin master
> > >
> > > On commit you will be given the change to properly format the message
> of
> > > the commit. We can use magic github words in the commit to
> automatically
> > > close the PR on commit.
> > > I will add examples for the messages and the auto close
> > >
> > > Wilfred
> > >
> > > On Sun, 26 Apr 2020 at 05:01, Wangda Tan <[email protected]> wrote:
> > >
> > > > This looks reasonable to me. If everybody agrees, we should add it to
> > the
> > > > dev doc.
> > > >
> > > > Thanks,
> > > > Wangda
> > > >
> > > > On Mon, Apr 13, 2020 at 11:56 PM Wilfred Spiegelenburg <
> > > > [email protected]> wrote:
> > > >
> > > >> Hi,
> > > >>
> > > >> We have been using the github squash and commit button to merge
> > requests
> > > >> for a most of our commits. I have noticed a problem with that usage.
> > The
> > > >> commit that is added by GitHub will be attributed to the person that
> > > >> opened
> > > >> the PR (that is good and correct). However the committer is set to
> > > github.
> > > >> This means that we cannot track back a commit to a committer unless
> > you
> > > >> use
> > > >> the github UI and open the PR. The code that is committed into the
> > repo
> > > is
> > > >> also not signed off by the person performing the commit but by
> using a
> > > >> general github signature.
> > > >>
> > > >> As an example [YUNIKORN-85] shows the following commit log entry:
> > > >> -+-+-+-+-
> > > >> Author:     Tao Yang <[email protected]>
> > > >> AuthorDate: Sat Apr 11 01:55:41 2020 +0800
> > > >> Commit:     GitHub <[email protected]>
> > > >> CommitDate: Fri Apr 10 10:55:41 2020 -0700
> > > >> -+-+-+-+-
> > > >>
> > > >> And on the UI it just shows Tao committed the change while Weiwei
> was
> > > the
> > > >> person that merged. You cannot find the correct detail unless you
> dig
> > > into
> > > >> the original PR on github itself.
> > > >>
> > > >> -+-+-+-+-
> > > >> [YUNIKORN-85] Improve recovery performance by querying all pods once
> > … …
> > > >> TaoYang526 committed 4 days ago
> > > >> -+-+-+-+-
> > > >>
> > > >> Because of this I already switched back to a manual squash and
> commit
> > of
> > > >> the changes setting the author etc. That shows up correctly in the
> > logs:
> > > >> -+-+-+-+-
> > > >> Author:     Weiwei Yang <[email protected]>
> > > >> AuthorDate: Fri Apr 10 02:17:16 2020 +1000
> > > >> Commit:     Wilfred Spiegelenburg <[email protected]>
> > > >> CommitDate: Fri Apr 10 02:17:16 2020 +1000
> > > >> -+-+-+-+-
> > > >> And also in the github UI:
> > > >> -+-+-+-+-
> > > >> [YUNIKORN-72] data race in unit test (#96) …
> > > >> yangwwei authored and wilfred-s committed 5 days ago
> > > >> -+-+-+-+-
> > > >>
> > > >> I want to propose that we all go back to that way so we do not lose
> > > >> the information of whom committed and get the correct signatures on
> > the
> > > >> committed code.
> > > >>
> > > >> Please let me know if this is acceptable.
> > > >>
> > > >> Wilfred
> > > >>
> > > >
> > >
> >
>

Reply via email to