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.

OpenStack-dev mailing list

Reply via email to