On Sat, Jan 23, 2016 at 6:39 AM, Gijs Kruitbosch <gijskruitbo...@gmail.com> wrote:
> On 22/01/2016 20:52, 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]. >> > > There are two reasons I disagree with this, both of which I believe to be > distinct from Boris' reasons: > > * "depends" is fuzzy and does not always lead to try or build failure. For > an example, I now have review for 80% of my patches on bug 1219215, but I > need to wait to land that until bug 1241275 gets a patch, lands, etc., > otherwise I will regress window dragging on Windows. To the best of my > knowledege we do not have try coverage for this, in part (I think) because > doing "native" aero snap dragging within mochitest isn't possible. The code > I'm touching won't conflict with the patch in bug 1241275, and anyway, a > finished patch wasn't available when I wrote patches for 1219215. > > * Historically, mozreview has been... less than amazing... about pushing > draft commits with multiple different bug numbers and treating them "as I > would expect" (ie, separate bug numbers go on separate bugs, not all in one > parent review. If it doesn't know what to do, it should ask with a warning, > or do nothing, not blunder ahead and mess things up.). I have developed the > reflex to always re-'up' to fx-team tip in my local repo before starting a > new patch - even if it's related to the other patch I'm writing - simply to > avoid accidentally pushing reviews onto the wrong bugzilla bug, which is > impossible to undo anyway, and sometimes messes with other bugs' state in > ways that are painful to manually recover. These are known problems and there is tons of room to improve. We made the decision to prioritize Q4 work around UI warts. With Git support for MozReview liking landing next week, this issue will likely be raised by a whole new crop of users, further reminding us of how painful it is. I think there are some low hanging fruits we can deploy in the next few weeks to make this much better. _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
