Open Stack

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.


I've gotten used to this. Typically when I review a new patch set I look
for my comments and make sure they were addressed. Then I go back to
compare with the base revision and look through the patch again. It's
quicker this time since I remember what it was about.

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'?


A developer updating a patch is going to want to test the change using the
latest master and not an old buggy version, so before you make your changes
you rebase and -R isn't going to make a difference. You could be really
considerate and re-rebase on the original parent. (Or you could be lazy and
not test your changes locally with the latest master.)

When you have a chain of commits rebasing gets more complicated since
gerrit shows a dependent review as out of date if the parent review is
changed in any way, and if there's a merge conflict far down the chain you
have to rebase the whole chain.

Rebasing the patch on master does make one thing easier -- if you want to
download the patch and try it out and your newer environment (tox or
devstack for example) doesn't work with the patch repo's old environment
you'll need to rebase first to get it to work.

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/5b7cf915/attachment.html>