You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Francis Chuang <fr...@apache.org> on 2019/01/03 22:37:06 UTC

Disabling of "merge commits" and "squash merge" on Github

Yesterday, Andrei brought to my attention that my attempt to merge 
Sergey's PR [1] using the default merge strategy (create a merge commit) 
on Github broke the linear history of the calcite repository.

I have since removed the commit and merged the PR using a rebase.

I have also tested the merge behaviors on a test repo on Github. The 
"Rebase and merge" strategy will replicate what we are currently doing 
using the git client to preserve linear history.

I have asked infra to disable the "create a merge commit" and "squash 
merge" strategies [2] on all our repositories.

The merge button on Github should now be safe to use. In the event that 
there is a conflict and Github is not able to perform the rebase, 
committers should manually rebase the commit(s) and merge the PR using 
the git client.

[1] https://github.com/apache/calcite/pull/772
[2] https://issues.apache.org/jira/browse/INFRA-17541

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Francis Chuang <fr...@apache.org>.
I tested the "squash merge" strategy on a test repo on Github yesterday. 
Unfortunately, the author of the original commit is not preserved. The 
author will be set to the person who merges the commit and the committer 
is set to Github.

Francis

On 5/01/2019 7:40 am, Michael Mior wrote:
> That comment makes it sound like it will be the authorship information for
> whoever presses the button, which is not what we want. If that's the case,
> keeping the button disabled makes sense to me.
> 
> --
> Michael Mior
> mmior@apache.org
> 
> 
> Le ven. 4 janv. 2019 à 13:14, Vladimir Sitnikov <si...@gmail.com>
> a écrit :
> 
>> Michael> If the squash merge button on GitHub keeps authorship
>>
>> According to
>> https://github.com/isaacs/github/issues/1368#issuecomment-427647139
>> it will set author name/email from GitHub details.
>>
>> Vladimir
>>
> 


Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Michael Mior <mm...@apache.org>.
That comment makes it sound like it will be the authorship information for
whoever presses the button, which is not what we want. If that's the case,
keeping the button disabled makes sense to me.

--
Michael Mior
mmior@apache.org


Le ven. 4 janv. 2019 à 13:14, Vladimir Sitnikov <si...@gmail.com>
a écrit :

> Michael> If the squash merge button on GitHub keeps authorship
>
> According to
> https://github.com/isaacs/github/issues/1368#issuecomment-427647139
> it will set author name/email from GitHub details.
>
> Vladimir
>

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Vladimir Sitnikov <si...@gmail.com>.
Michael> If the squash merge button on GitHub keeps authorship

According to https://github.com/isaacs/github/issues/1368#issuecomment-427647139
it will set author name/email from GitHub details.

Vladimir

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Michael Mior <mm...@apache.org>.
I wasn't advocating this, just explaining what has been done in the past.
If the squash merge button on GitHub keeps authorship, I would prefer to
keep that enabled.

--
Michael Mior
mmior@apache.org

Le ven. 4 janv. 2019 à 12:06, Julian Hyde <jh...@apache.org> a écrit :

>
>
> > On Jan 4, 2019, at 6:37 AM, Michael Mior <mm...@apache.org> wrote:
> >
> > What we've done in the past is to add the original author's name in
> > parentheses to the commit message. Not ideal, but better than losing the
> > attribution entirely.
>
> Let’s not do that. The author’s name must be in the git commit’s “author”
> field.
>
> Our "author’s name in parentheses” practice serves some other purposes -
> e.g. giving credit to new contributors in the release notes - but it’s not
> a panacea.
>
> Julian
>
>

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Julian Hyde <jh...@apache.org>.

> On Jan 4, 2019, at 6:37 AM, Michael Mior <mm...@apache.org> wrote:
> 
> What we've done in the past is to add the original author's name in
> parentheses to the commit message. Not ideal, but better than losing the
> attribution entirely.

Let’s not do that. The author’s name must be in the git commit’s “author” field.

Our "author’s name in parentheses” practice serves some other purposes - e.g. giving credit to new contributors in the release notes - but it’s not a panacea.

Julian


Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Vladimir Sitnikov <si...@gmail.com>.
>I guess that's not possible with Github.

AFAIK all merge options (rebase, squash, merge) at GitHub
automatically keep PR author.

Vladimir

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Stamatis Zampetakis <za...@gmail.com>.
Thanks for the clarification Francis.

Using the git client it is possible to preserve the original author (even
in the case of a squash merge) by setting the --author parameter during the
commit.

Example: git commit --author="Stamatis Zampetakis <za...@gmail.com>"

I guess that's not possible with Github.

Στις Παρ, 4 Ιαν 2019 στις 3:38 μ.μ., ο/η Michael Mior <mm...@apache.org>
έγραψε:

> What we've done in the past is to add the original author's name in
> parentheses to the commit message. Not ideal, but better than losing the
> attribution entirely. I think it would be good to make an effort to ask the
> contributor to do the squash themselves but in some cases, the PR is ready
> to merge but the original contributor is no longer available. This
> shouldn't hold us back. That said, I don't have a particular problem with
> disabling the button on GitHub.
>
> --
> Michael Mior
> mmior@apache.org
>
>
> Le ven. 4 janv. 2019 à 03:37, Francis Chuang <fr...@apache.org> a
> écrit :
>
> > Hey Stamatis,
> >
> > Squash merge on Github does preserve linear history. The commits are
> > squashed into a new commit and then added to master.
> >
> > The problem with squash commit merge on Github is that the author of the
> > squashed commit is set to the person who merged the PR and the committer
> > is set to Github.
> >
> > This is not ideal, as the original author of the commits would not be
> > given credit for their work.
> >
> > I think the best solution would be for PR authors to squash their
> > commits locally and force push to their branch once work has been
> > finalized.
> >
> > Francis
> >
> > On 4/01/2019 7:11 pm, Stamatis Zampetakis wrote:
> > > Hi Francis,
> > >
> > > I had the impression that squash merge also retains the linear history.
> > > I am always performing these operations using the git client; is the
> > result
> > > different when using Github?
> > >
> > > Best,
> > > Stamatis
> > >
> > > Στις Πέμ, 3 Ιαν 2019 στις 11:37 μ.μ., ο/η Francis Chuang <
> > > francischuang@apache.org> έγραψε:
> > >
> > >> Yesterday, Andrei brought to my attention that my attempt to merge
> > >> Sergey's PR [1] using the default merge strategy (create a merge
> commit)
> > >> on Github broke the linear history of the calcite repository.
> > >>
> > >> I have since removed the commit and merged the PR using a rebase.
> > >>
> > >> I have also tested the merge behaviors on a test repo on Github. The
> > >> "Rebase and merge" strategy will replicate what we are currently doing
> > >> using the git client to preserve linear history.
> > >>
> > >> I have asked infra to disable the "create a merge commit" and "squash
> > >> merge" strategies [2] on all our repositories.
> > >>
> > >> The merge button on Github should now be safe to use. In the event
> that
> > >> there is a conflict and Github is not able to perform the rebase,
> > >> committers should manually rebase the commit(s) and merge the PR using
> > >> the git client.
> > >>
> > >> [1] https://github.com/apache/calcite/pull/772
> > >> [2] https://issues.apache.org/jira/browse/INFRA-17541
> > >>
> > >
> >
> >
>

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Michael Mior <mm...@apache.org>.
What we've done in the past is to add the original author's name in
parentheses to the commit message. Not ideal, but better than losing the
attribution entirely. I think it would be good to make an effort to ask the
contributor to do the squash themselves but in some cases, the PR is ready
to merge but the original contributor is no longer available. This
shouldn't hold us back. That said, I don't have a particular problem with
disabling the button on GitHub.

--
Michael Mior
mmior@apache.org


Le ven. 4 janv. 2019 à 03:37, Francis Chuang <fr...@apache.org> a
écrit :

> Hey Stamatis,
>
> Squash merge on Github does preserve linear history. The commits are
> squashed into a new commit and then added to master.
>
> The problem with squash commit merge on Github is that the author of the
> squashed commit is set to the person who merged the PR and the committer
> is set to Github.
>
> This is not ideal, as the original author of the commits would not be
> given credit for their work.
>
> I think the best solution would be for PR authors to squash their
> commits locally and force push to their branch once work has been
> finalized.
>
> Francis
>
> On 4/01/2019 7:11 pm, Stamatis Zampetakis wrote:
> > Hi Francis,
> >
> > I had the impression that squash merge also retains the linear history.
> > I am always performing these operations using the git client; is the
> result
> > different when using Github?
> >
> > Best,
> > Stamatis
> >
> > Στις Πέμ, 3 Ιαν 2019 στις 11:37 μ.μ., ο/η Francis Chuang <
> > francischuang@apache.org> έγραψε:
> >
> >> Yesterday, Andrei brought to my attention that my attempt to merge
> >> Sergey's PR [1] using the default merge strategy (create a merge commit)
> >> on Github broke the linear history of the calcite repository.
> >>
> >> I have since removed the commit and merged the PR using a rebase.
> >>
> >> I have also tested the merge behaviors on a test repo on Github. The
> >> "Rebase and merge" strategy will replicate what we are currently doing
> >> using the git client to preserve linear history.
> >>
> >> I have asked infra to disable the "create a merge commit" and "squash
> >> merge" strategies [2] on all our repositories.
> >>
> >> The merge button on Github should now be safe to use. In the event that
> >> there is a conflict and Github is not able to perform the rebase,
> >> committers should manually rebase the commit(s) and merge the PR using
> >> the git client.
> >>
> >> [1] https://github.com/apache/calcite/pull/772
> >> [2] https://issues.apache.org/jira/browse/INFRA-17541
> >>
> >
>
>

Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Francis Chuang <fr...@apache.org>.
Hey Stamatis,

Squash merge on Github does preserve linear history. The commits are 
squashed into a new commit and then added to master.

The problem with squash commit merge on Github is that the author of the 
squashed commit is set to the person who merged the PR and the committer 
is set to Github.

This is not ideal, as the original author of the commits would not be 
given credit for their work.

I think the best solution would be for PR authors to squash their 
commits locally and force push to their branch once work has been finalized.

Francis

On 4/01/2019 7:11 pm, Stamatis Zampetakis wrote:
> Hi Francis,
> 
> I had the impression that squash merge also retains the linear history.
> I am always performing these operations using the git client; is the result
> different when using Github?
> 
> Best,
> Stamatis
> 
> Στις Πέμ, 3 Ιαν 2019 στις 11:37 μ.μ., ο/η Francis Chuang <
> francischuang@apache.org> έγραψε:
> 
>> Yesterday, Andrei brought to my attention that my attempt to merge
>> Sergey's PR [1] using the default merge strategy (create a merge commit)
>> on Github broke the linear history of the calcite repository.
>>
>> I have since removed the commit and merged the PR using a rebase.
>>
>> I have also tested the merge behaviors on a test repo on Github. The
>> "Rebase and merge" strategy will replicate what we are currently doing
>> using the git client to preserve linear history.
>>
>> I have asked infra to disable the "create a merge commit" and "squash
>> merge" strategies [2] on all our repositories.
>>
>> The merge button on Github should now be safe to use. In the event that
>> there is a conflict and Github is not able to perform the rebase,
>> committers should manually rebase the commit(s) and merge the PR using
>> the git client.
>>
>> [1] https://github.com/apache/calcite/pull/772
>> [2] https://issues.apache.org/jira/browse/INFRA-17541
>>
> 


Re: Disabling of "merge commits" and "squash merge" on Github

Posted by Stamatis Zampetakis <za...@gmail.com>.
Hi Francis,

I had the impression that squash merge also retains the linear history.
I am always performing these operations using the git client; is the result
different when using Github?

Best,
Stamatis

Στις Πέμ, 3 Ιαν 2019 στις 11:37 μ.μ., ο/η Francis Chuang <
francischuang@apache.org> έγραψε:

> Yesterday, Andrei brought to my attention that my attempt to merge
> Sergey's PR [1] using the default merge strategy (create a merge commit)
> on Github broke the linear history of the calcite repository.
>
> I have since removed the commit and merged the PR using a rebase.
>
> I have also tested the merge behaviors on a test repo on Github. The
> "Rebase and merge" strategy will replicate what we are currently doing
> using the git client to preserve linear history.
>
> I have asked infra to disable the "create a merge commit" and "squash
> merge" strategies [2] on all our repositories.
>
> The merge button on Github should now be safe to use. In the event that
> there is a conflict and Github is not able to perform the rebase,
> committers should manually rebase the commit(s) and merge the PR using
> the git client.
>
> [1] https://github.com/apache/calcite/pull/772
> [2] https://issues.apache.org/jira/browse/INFRA-17541
>