<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 14, 2016 at 1:54 AM, Gijs Kruitbosch <span dir="ltr"><<a href="mailto:gijskruitbosch@gmail.com" target="_blank">gijskruitbosch@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi,<br>
<br>
First things first: I support that we're eventually moving to autoland and getting rid of "manual" integration trees. I will also survive when fx-team goes away [soonish].<br>
<br>
However, some issues with the reasoning here...<span class=""><br>
<br>
On 13/07/2016 22:10, Gregory Szorc wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
First, *I would like to encourage Firefox developers to prepare for the future by stopping to use fx-team*. Don't `hg pull` from fx-team: base your work on mozilla-central instead. Use MozReview + Autoland to land your commits (they will land in the integration/autoland repo).<br>
</blockquote></span>
This is not possible for all commits (*cough* security bugs *cough*). I would really like that to get fixed, but until it does, I continue to be, to borrow some song lyrics "stuck in the middle" between a painful manual upload process and one I shouldn't use for the thing I'm doing.<br></blockquote><div><br></div><div>Security bugs are special. Since the majority of commits aren't tied to security bugs, I don't think we should let a minority use case have tyranny over the majority. So let's ignore them for purposes of this discussion.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Even where it *is* possible to use autoland, it's often just more hassle to use autoland. Folks were surprised recently when I pointed out my dashboards in mozreview were useless because I have hundreds and hundreds of "open" review requests. This is why. Common situation:<br></blockquote><div><br></div><div>FWIW MozReview review requests are closed automatically when using autoland, making your dashboard usable.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
1) push commits to mozreview. Get r+ with 5 nits to fix some days later<br>
2) my tree has moved on since then. I rebase my patch onto fx-team tip, fix the nits, hg commit --amend, replace "?" with "=" in the commit message.<br>
<br>
Now I have two options:<br>
a) push to fx-team<br>
b) push to mozreview to update the patch there, then select the review link in the terminal (painful on Windows), manually copy-paste into new tab in web browser, hope mozreview hasn't decided to re-request any reviews, click the small [5!] button at the top of the diff view, close all the outstanding issues manually, click the "finish" button to actually submit those changes if I made any comments, but even if not, manually refresh the page because the autoland option is still going to be greyed out (hopefully bug 1253552 is fixing this?), use UI to autoland. This is also confusing because the issue summary of a kid review request (e.g. <a href="https://reviewboard.mozilla.org/r/62876/" rel="noreferrer" target="_blank">https://reviewboard.mozilla.org/r/62876/</a> ) has an Automation entry in the top navigation bar, but the diff view (e.g. <a href="https://reviewboard.mozilla.org/r/62876/diff/" rel="noreferrer" target="_blank">https://reviewboard.mozilla.org/r/62876/diff/</a> ) does not, and so I usually spend at least some time looking for a button that's not there.<br>
<br>
Not to be mean, but the second workflow is pretty rubbish. It takes a lot longer and requires a lot of mousework and context switches. 3 guesses which one I therefore end up picking 90% of the time... :-)<br>
<br></blockquote><div><br></div><div>You are correct that 2nd workflow is pretty poor and that there are some UI warts around the "automation" entry in the navigation bar.<br><br></div><div>I think we can make things a lot better by streamlining the autoland workflow. For example, we can add an --autoland argument so pushing to MozReview initiates landing (if appropriate), saving you the time from opening your browser. Aside from the obvious UX advantages, it also captures the final interdiff in MozReview, sending an update to Bugzilla and giving people an easy way to see what the rebase changed. If anyone wants to audit things, we just made that a lot easier.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Conversely, if I get a straight r+ with no nits, I almost always use autoland because it's a lot less work: I open the bug with the link from my bugmail, click the link there to go to mozreview (end up on diff view with no automation menu), find the tiny tiny "all changes" link to go to the parent review request, and autoland immediately from there. This could still be a smoother process, but it's less faff than rebasing in the terminal and pushing to fx-team (and making sure the thing I have locally is identical to the thing I pushed to mozreview and got r+ on!).<br>
<br>
(even there there's one more downside, though: I end up periodically having to go through my draft csets and pruning all the ones that already landed manually - it'd be neat if we had a mercurial extension that could make this semi-automatic.)<br></blockquote><div><br></div><div>I'm working on it. If you use the evolve extension, things should "just work" in the next few months. For non-evolve users (including Git users), I may write a command that tries to find copies by scanning the MozReview-Commit-ID metadata or something.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Recommendations in decreasing order of effectiveness:<br>
1) with level 3 commit access it should be possible for me to push-to-get-this-autolanded from the commandline, much like I can push to try. I should not need my web browser at that stage of the process.<br></blockquote><div><br></div><div>I agree.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
2) don't block autoland UI in mozreview on open issues, just warn rather than disabling it outright.<br></blockquote><div><br></div><div>Please file a bug on this if there isn't already.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
3) if hard to do, at least provide the autoland link from diff views, and just make mozreview do the 'Hi, I'm gonna push these things' collating all items, maybe with a big red warning if there's more than 1 cset in there.<br></blockquote><div><br></div><div>Yeah, I'm not sure why the automation entry isn't on the diffs pages. I agree it should be there. A bug should be filed.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
4) provide fix/drop all remaining issues buttons on both the diff and issue summary page<br>
5) make navigating issues in the diff easier. Right now if I want to resolve issues I have to manually look for the dark-blue-on-green blobs in the side of the diff view, 'collect them all' and click "Fix"/"Drop" in all of them (and there's no running count, so I have to go all the way back up to the top to know if I'm done - so I never do this. Or click the [3!] button at the top of the diff view to take me to the "issue summary", wait ages for that page to load, and then be able to close issues in quicker succession - but the extra page load makes this feel slow/clunky, too.***<br></blockquote><div><br></div><div>I think this is being addressed by the work to show comments inline.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
6) make the 'review all changes' link BIGGER. The click target is too small, and it's in the middle of the page so it's not easy to move a pointer to either.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
It is true you won't live on the bleeding edge. But I argue this is mostly a good thing because the bleeding edge gets you, well, bloody. Central has higher stability guarantees so you won't hit build or unexpected test failures as frequently (due to pulling a bad changeset from fx-team). If you've ever based work on a bad changeset, pushed to Try, and lost hours after realizing the base changeset was bad, you know how important stability can be.<br>
</blockquote></span>
This almost never happens on fx-team. On inbound, yes. But almost everything that lands on fx-team is frontend-only, and so builds almost never break, and the test results I care about are normally fine, too. So this isn't really a big downside of fx-team IME.<span class=""><br>
<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Second, I would like to hear what objections or concerns there are around eliminating fx-team. If fx-team goes away, your options will be to use Autoland (preferred) or just start using inbound instead (until inbound goes away, anyway - but this will be a while).<br>
</blockquote></span>
In addition to the above: stability for pushing. inbound breaks more often so inbound is closed more often, so it's harder for me to push to inbound than it is to fx-team.<br></blockquote><div><br></div><div>So it sounds like most of your complaints are about MozReview UX. A lot of these complaints aren't new to the MozReview team. But you may want to needinfo mcote on bugs to see if they are on the team's radar.<br><br></div><div>It doesn't sound like your concerns block the removal of fx-team however: if you don't want to use MozReview+Autoland, you can just substitute inbound for fx-team and continue working like you currently do.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
<br>
***<br>
Just generally, it feels like mozreview is full of loading indicators. Everywhere. Even things that should just be instantaneous, like if you click "Finish review..." first you get a "Loading..." blob, before the dialog shows up. When the dialog shows up, then *that* has *another* loading blob while it's collecting all the issues. The dialog should just show up immediately - all the data it initially shows is already in the page that's "below" the dialog, so I have no idea what it's loading that it needed to wait for... I could sympathise with it having to fetch issues from the network (the second "loading" blob in the dialog) but even there it feels like it should have an optimistic cache of the data locally so that you don't feel like you're. waiting. all. the. time. Then if you click "Close" the dialog doesn't dismiss immediately, you get some kind of slow fade-from-black animation. If you actually submit a review, that gets you another "Loading..." blob but the button to submit it isn't immediately disabled, so you can click it twice, which leads to "interesting" "HTTP 0" (wat) errors. Generally it feels like all the animation durations should be at least halved if not gotten rid of entirely</blockquote><div><br></div><div>I wonder if this is a consequence of you being on a different continent from the server in California...<br></div><div> </div></div></div></div>