On 1/22/16 3:52 PM, Gregory Szorc wrote:
I would say that pushing cherry-picked commits for review that depend on
other commits not in the commit's ancestry is just wrong. If you pushed
this to Try, it would fail. So why are you pushing a "bad" commit/tree for
review? If your commits depend on something else, that something else
should be in the ancestry [and available to MozReview to inspect].
I'm going to assume this is a non-rhetorical question.
Here's a concrete example of a reason to do this. Say I have two
patches for two separate bugs. They're totally unrelated to each other
in general, but they happen to both need to touch the same spot of code,
possibly in the same way, but possibly in slightly different ways). So
the patches conflict with each other, effectively.
Now I have several options:
1) Develop the patches on separate branches (or on separate repos),
request separate reviews on them, whichever one loses the review/land
race needs to get updated in mozreview before getting pushed.
2) Develop the patches on the same branch, request reviews on them, if
the one that's "second" gets review first, reorder the commits, making
the minor changes needed to handle that one conflict, land the thing
that has review, update the other one in mozreview.
3) Develop the patches on the same branch, request reviews on them,
wait until the "first" one has reviews before landing either one.
In practice, I believe option 3 is the worst one for the situation I
described, because it gates an unrelated bug on another bug. And this
situation happens all the time. I'd estimate that at least 10% of my
patches have self-conflicts of the type I described above.
Option 2 is nice in needing a bit less overhead in terms of working
trees or rebuilds on branch-switches. It also just happens
automatically quite often, unless you're using one branch per bug
always. I believe you're saying this option is unsupported by MozReview
if the actual branch is pushed (because it's two separate bugs), so the
natural way to use MozReview here would be to cherry-pick.
Option 1 just happens if you use one branch per bug, or just happen to
work in a different local repo and don't realize you have the
self-conflict. Happens all the time, again. In practice it will lead to
precisely the "cherrypicked" setup Ehsan described: as soon as one patch
lands, the other will behave as if it had been cherrypicked.
The ultimate goal is to
push things as they need to land
This is a ... pretty vague concept. In practice, the way it works is
you write a bunch of code for nominally unrelated bugs, some of it
possibly with non-obvious dependencies or self-conflicts or both,
possibly on one branch, possibly on several branches, possibly on
several branches with multiple bugs per branch. Then you ask for
reviews on all of the unrelated things in parallel and wait hours to
months depending on the reviewers. Then in the order you get the
reviews you cherrypick things onto that day's tip, fix merge issues as
needed, run a try push if you're not confident enough that things are
still OK, and then push.
-Boris
_______________________________________________
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
