You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@impala.apache.org by Tim Armstrong <ta...@cloudera.com> on 2017/12/11 19:43:11 UTC
Switch gerrit merge strategy to "rebase always"?
We recently had a bad merge that was allowed by the cherry-pick merge
strategy merging a simple without its ancestor (since they didn't change
any nearby lines):
https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
It looks like the "rebase always" merge strategy avoids this by always
trying to rebase and merge the whole chain of commits. Does anyone have any
objections or thoughts about switching to this strategy?
Re: Switch gerrit merge strategy to "rebase always"?
Posted by Jim Apple <jb...@cloudera.com>.
OK, sgtm
On Mon, Dec 11, 2017 at 1:07 PM, Tim Armstrong <ta...@cloudera.com>
wrote:
> AFAIK there isn't a version of cherry-pick like that (which only
> cherry-picks commits if their parent is in the target branch).
>
> "Rebase always" should include the same footer as "cherry pick", according
> to that link you provided. They summarise it as "Rebase Always can be
> considered similar to Cherry Pick, but with the important distinction that
> Rebase Always does not ignore dependencies."
>
> On Mon, Dec 11, 2017 at 11:55 AM, Jim Apple <jb...@cloudera.com> wrote:
>
> > Is there a merge strategy that just doesn't allow commit chains longer
> than
> > 1?
> >
> > Also
> > https://gerrit-review.googlesource.com/Documentation/project-
> > configuration.html
> > says
> >
> > "When cherry picking a change, Gerrit automatically appends onto the end
> of
> > the commit message a short summary of the change’s approvals, and a URL
> > link back to the change on the web. The committer header is also set to
> the
> > submitter, while the author header retains the original patch set
> author."
> >
> > I love that short message. It's useful to be able to easily see the code
> > review comments and reviewer names.
> >
> > On Mon, Dec 11, 2017 at 11:43 AM, Tim Armstrong <tarmstrong@cloudera.com
> >
> > wrote:
> >
> > > We recently had a bad merge that was allowed by the cherry-pick merge
> > > strategy merging a simple without its ancestor (since they didn't
> change
> > > any nearby lines):
> > > https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d
> > > 28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
> > >
> > > It looks like the "rebase always" merge strategy avoids this by always
> > > trying to rebase and merge the whole chain of commits. Does anyone have
> > any
> > > objections or thoughts about switching to this strategy?
> > >
> >
>
Re: Switch gerrit merge strategy to "rebase always"?
Posted by Tim Armstrong <ta...@cloudera.com>.
AFAIK there isn't a version of cherry-pick like that (which only
cherry-picks commits if their parent is in the target branch).
"Rebase always" should include the same footer as "cherry pick", according
to that link you provided. They summarise it as "Rebase Always can be
considered similar to Cherry Pick, but with the important distinction that
Rebase Always does not ignore dependencies."
On Mon, Dec 11, 2017 at 11:55 AM, Jim Apple <jb...@cloudera.com> wrote:
> Is there a merge strategy that just doesn't allow commit chains longer than
> 1?
>
> Also
> https://gerrit-review.googlesource.com/Documentation/project-
> configuration.html
> says
>
> "When cherry picking a change, Gerrit automatically appends onto the end of
> the commit message a short summary of the change’s approvals, and a URL
> link back to the change on the web. The committer header is also set to the
> submitter, while the author header retains the original patch set author."
>
> I love that short message. It's useful to be able to easily see the code
> review comments and reviewer names.
>
> On Mon, Dec 11, 2017 at 11:43 AM, Tim Armstrong <ta...@cloudera.com>
> wrote:
>
> > We recently had a bad merge that was allowed by the cherry-pick merge
> > strategy merging a simple without its ancestor (since they didn't change
> > any nearby lines):
> > https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d
> > 28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
> >
> > It looks like the "rebase always" merge strategy avoids this by always
> > trying to rebase and merge the whole chain of commits. Does anyone have
> any
> > objections or thoughts about switching to this strategy?
> >
>
Re: Switch gerrit merge strategy to "rebase always"?
Posted by Jim Apple <jb...@cloudera.com>.
Is there a merge strategy that just doesn't allow commit chains longer than
1?
Also
https://gerrit-review.googlesource.com/Documentation/project-configuration.html
says
"When cherry picking a change, Gerrit automatically appends onto the end of
the commit message a short summary of the change’s approvals, and a URL
link back to the change on the web. The committer header is also set to the
submitter, while the author header retains the original patch set author."
I love that short message. It's useful to be able to easily see the code
review comments and reviewer names.
On Mon, Dec 11, 2017 at 11:43 AM, Tim Armstrong <ta...@cloudera.com>
wrote:
> We recently had a bad merge that was allowed by the cherry-pick merge
> strategy merging a simple without its ancestor (since they didn't change
> any nearby lines):
> https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d
> 28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
>
> It looks like the "rebase always" merge strategy avoids this by always
> trying to rebase and merge the whole chain of commits. Does anyone have any
> objections or thoughts about switching to this strategy?
>
Re: Switch gerrit merge strategy to "rebase always"?
Posted by Tim Armstrong <ta...@cloudera.com>.
Ok, I went ahead and changed it in gerrit. Let me know if you have any
problems with it.
On Mon, Dec 11, 2017 at 11:58 AM, Philip Zeyliger <ph...@cloudera.com>
wrote:
> Seems like it's the right thing to do.
>
> On Mon, Dec 11, 2017 at 11:43 AM, Tim Armstrong <ta...@cloudera.com>
> wrote:
>
> > We recently had a bad merge that was allowed by the cherry-pick merge
> > strategy merging a simple without its ancestor (since they didn't change
> > any nearby lines):
> > https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d
> > 28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
> >
> > It looks like the "rebase always" merge strategy avoids this by always
> > trying to rebase and merge the whole chain of commits. Does anyone have
> any
> > objections or thoughts about switching to this strategy?
> >
>
Re: Switch gerrit merge strategy to "rebase always"?
Posted by Philip Zeyliger <ph...@cloudera.com>.
Seems like it's the right thing to do.
On Mon, Dec 11, 2017 at 11:43 AM, Tim Armstrong <ta...@cloudera.com>
wrote:
> We recently had a bad merge that was allowed by the cherry-pick merge
> strategy merging a simple without its ancestor (since they didn't change
> any nearby lines):
> https://lists.apache.org/thread.html/ee81ee3e396a9a7b1214d92d713a2d
> 28f2f1f7058184504ebc399170@%3Cdev.impala.apache.org%3E
>
> It looks like the "rebase always" merge strategy avoids this by always
> trying to rebase and merge the whole chain of commits. Does anyone have any
> objections or thoughts about switching to this strategy?
>