I will try this as well.

Thanks
Sunil

On Thu, Jun 11, 2020 at 12:16 AM Weiwei Yang <[email protected]> wrote:

> Hi Wilfred
>
> I think the problem with the PR merge is due to
>
> https://help.github.com/en/github/setting-up-and-managing-your-github-user-account/setting-your-commit-email-address#setting-your-commit-email-address-on-github
> ,
> I think if the PR submitter doesn't have this box checked in their github
> setting, the email settings will be correct:
>
> Keep my email addresses private
>
> otherwise, github hides the email address. I just did a test with
> https://github.com/apache/incubator-yunikorn-release/pull/7. Please take a
> look if this is the case.
>
> Thanks!
>
> On Tue, Jun 2, 2020 at 12:24 AM Wilfred Spiegelenburg <[email protected]
> >
> wrote:
>
> > The way github commits has been the same for a long time. It has been the
> > way github commits since they added it to the UI. From a github
> perspective
> > they cannot or will not fix this, it is linked to authentication,
> > authorisation and code sign off.
> > We will never get the committer set to anything but "Github <
> > [email protected]>" if you use the web UI.
> > I have gone back over more than 2 years of discussions and numerous
> support
> > tickets logged by other groups with github and it has not changed.
> >
> > It is more than just the email address that is not associated. Editing
> and
> > getting the message layout is also more difficult.
> > I am working on a way to almost fully script the process: pulling the
> > change(s), creating a local branch, squash merge into master etc. That
> > would just leave the writing of the message locally without the need to
> do
> > anything "manually"
> >
> > Wilfred
> >
> > On Tue, 2 Jun 2020 at 06:27, Weiwei Yang <[email protected]> wrote:
> >
> > > Hi Wilfred
> > >
> > > I just tried to follow these steps to push a commit. (I was always
> using
> > > the github merge PR button)
> > > The manual steps are quite time consuming, I think we need to fix this
> > > issue in github.
> > > IIUC, the problematic commits are caused by the PR submitter did not
> > > associate their email address while submitting the PR, if this is the
> > case,
> > > we could just ensure the user name and email are correctly set?
> > >
> > >
> > > On Mon, Jun 1, 2020 at 9:28 AM Weiwei Yang <[email protected]>
> wrote:
> > >
> > > > Hi Wilfred
> > > >
> > > > Thank you for putting this together. Agree to have some doc and
> > examples
> > > > and every committer should follow the same rule.
> > > > Moving on, it might also good to look at some auto-merge features,
> such
> > > as
> > > > leveraging github action or github bot to enforce these rules. That
> > might
> > > > be fun to look at as well.
> > > >
> > > > Weiwei
> > > >
> > > >
> > > > On Mon, Jun 1, 2020 at 8:41 AM 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