Open Stack

On 12/30/2014 11:47 AM, Jeremy Stanley wrote:
> On 2014-12-30 10:32:22 -0600 (-0600), Dolph Mathews wrote:
>> The default behavior, rebasing automatically, is the sane default
>> to avoid having developers run into unexpected merge conflicts on
>> new patch submissions.
> Please show me an example of this in the wild. I suspect a lot of
> reviewers are placing blame on this without due investigation.
I think you msiread the intention of what Dolph posted, Jeremy. What he 
is saying is that "automatic rebasing ensures that a submitted patch 
would not get a false negative on a rebase problem."  Or, to try to make 
it even clearer, the default behaviour forces the submitter to deal with 
rebase problems in their own sand box.
>
>> 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.)
> It already requires contributors to take manual action--it will not
> automatically rebase and then push that without additional steps.
What I would like to see is this:
1.  Rebase the last patch. (if possible)
2.  Submit the new patch

Now a reviewer can see the difference between what was actually 
submitted and the previous patch.

If step 1 fails (it often does using the git review option for diff 
between two versions of the patch)  just accept it and move on.

>
>> 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.
> The only thing -R is going to accomplish is people uploading changes
> which can never pass because they're merge-conflicting with the
> target branch.
It makes it clearer what the diff is without complicating it with 
unrelated changes, which is what David wants to make happen. Ideally, 
any user that did a -R  would immediately do a rebase as well, but that 
would complicate things for the reviewer.

This is a common problem, and not a trivial one to solve.