[openstack-dev] [all] Proper use of 'git review -R'

Dolph Mathews dolph.mathews at gmail.com
Tue Dec 30 16:32:22 UTC 2014


The default behavior, rebasing automatically, is the sane default to avoid
having developers run into unexpected merge conflicts on new patch
submissions.

But if git-review can check to see if a review already exists in gerrit
*before* doing the local rebase, I'd be in favor of it skipping the rebase
by default if the review already exists. Require developers to rebase
existing patches manually. (This is my way of saying I can't think of a
good answer to your question.)

While we're on the topic, it's also worth noting that --no-rebase becomes
critically important when a patch in the review sequence has already been
approved, because the entire series will be rebased, potentially pulling
patches out of the gate, clearing the Workflow+1 bit, and resetting the
gate (probably unnecessarily). A tweak to the default behavior would help
avoid this scenario.

-Dolph

On Tue, Dec 30, 2014 at 8:46 AM, David Kranz <dkranz at redhat.com> wrote:

> Many times when I review a revision of an existing patch, I can't see just
> the change from the previous version due to other rebases. The git-review
> documentation mentions this issue and suggests using -R to make life easier
> for reviewers when submitting new revisions. Can some one explain when we
> should *not* use -R after doing 'git commit --amend'? Or is using -R just
> something that should be done but many folks don't know about it?
>
> -David
>
> From git-review doc:
>
> -R, --no-rebase
> Do not automatically perform a rebase before submitting the
> change to Gerrit.
>
> When submitting a change for review, you will usually want it to
> be based on the tip of upstream branch in order to avoid possible
> conflicts. When amending a change and rebasing the new patchset,
> the Gerrit web interface will show a difference between the two
> patchsets which contains all commits in between. This may confuse
> many reviewers that would expect to see a much simpler differ‐
> ence.
>
>
> _______________________________________________
> OpenStack-dev mailing list
> OpenStack-dev at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141230/b5e4d28f/attachment.html>


More information about the OpenStack-dev mailing list