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

David Kranz dkranz at redhat.com
Tue Dec 30 17:31:35 UTC 2014


On 12/30/2014 11:37 AM, Jeremy Stanley wrote:
> On 2014-12-30 09:46:35 -0500 (-0500), David Kranz wrote:
> [...]
>> Can some one explain when we should *not* use -R after doing 'git
>> commit --amend'?
> [...]
>
> In the standard workflow this should never be necessary. The default
> behavior in git-review is to attempt a rebase and then undo it
> before submitting. If the rebase shows merge conflicts, the push
> will be averted and the user instructed to deal with those
> conflicts. Using -R will skip this check and allow you to push
> changes which can't merge due to conflicts.
>
>>  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.
> While not entirely incorrect, it could stand to be updated with
> slightly more clarification around the fact that git-review (since
> around 1.16 a few years ago) does not push an automatically rebased
> change for you unless you are using -F/--force-rebase.
>
> If you are finding changes which are gratuitously rebased, this is
> likely either from a contributor who does not use the recommended
> change update workflow, has modified their rebase settings or
> perhaps is running a very, very old git-review version.
Thanks for the replies. The rebases I was referring to are not 
gratuitous, they just make it harder for the reviewer. I take a few 
things away from this.

1. This is really a UI issue, and one that is experienced by many. What 
is desired is an option to look at different revisions of the patch that 
show only what the author actually changed, unless there was a conflict.

2. Using -R is dangerous unless you really know what you are doing. The 
doc string makes it sound like an innocuous way to help reviewers.

  -David





More information about the OpenStack-dev mailing list