Preparing for the retirement of fx-team

69 views
Skip to first unread message

Gregory Szorc

unread,
Jul 13, 2016, 5:10:38 PM7/13/16
to Firefox Dev, sheriffs owner, dev-version-control
Firefox contributors,

Firefox contributors have done a great job of adopting MozReview and Autoland (it appears you are more willing to use new tools than Platform contributors). Thank you.

The number of pushes to fx-team has always been lower than inbound (historically ~500 non-merge pushes/month compared to ~1500). With the Autoland service and repo, the number of pushes to fx-team has been declining. We're now sitting around 300 pushes/month to fx-team. This is lower than the number of pushes to the autoland repo, which is our preferred repo for landing going forward.

*Integration repositories don't exist in the long term repository management strategy for Firefox.* fx-team and inbound will eventually go away. The question is when.

fx-team's push volume is already at a point where I question its value. In addition, upcoming changes to accommodate Servo will be putting more load on Sheriffs and Firefox automation. It would be really nice if we could offset those increases by eliminating fx-team.

The purpose of this email is twofold.

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). Sheriffs have increased the frequency of merges to central. So it now takes hours for landings to get incorporated into central instead of upwards of a day (or more). 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.

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).

If there aren't major objections, I imagine fx-team could be eliminated by the end of next month. Although I'd like to see fx-team usage drop significantly before we disable it wholesale, as I prefer people make the transition on their own terms so we don't cause too much disruption.

Finally, this proposal has nothing to do with the relative importance of Firefox vs Gecko/Platform/Servo. This is mostly about planned changes to how the Firefox repositories are managed. The relatively low volume of fx-team and the general willingness of its users to adopt modern tools (namely MozReview+Autoland) make it logical to retire fx-team before inbound.

Thoughts?

Justin Dolske

unread,
Jul 13, 2016, 8:09:50 PM7/13/16
to Gregory Szorc, sheriffs owner, dev-version-control, Firefox Dev
On Wed, Jul 13, 2016 at 2:10 PM, Gregory Szorc <g...@mozilla.com> wrote:

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).

I'm not aware of any reason to specifically keep fx-team around... People might have an opinion on what blocks ultimately removing mozilla-inbound, but AFIAK anyone using fx-team can trivially switch to mozilla-inbound.

Justin

Gijs Kruitbosch

unread,
Jul 14, 2016, 4:55:01 AM7/14/16
to Gregory Szorc, Firefox Dev
Hi,

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].

However, some issues with the reasoning here...

On 13/07/2016 22:10, Gregory Szorc wrote:
> 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).
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.

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:

1) push commits to mozreview. Get r+ with 5 nits to fix some days later
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.

Now I have two options:
a) push to fx-team
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. https://reviewboard.mozilla.org/r/62876/ ) has an
Automation entry in the top navigation bar, but the diff view (e.g.
https://reviewboard.mozilla.org/r/62876/diff/ ) does not, and so I
usually spend at least some time looking for a button that's not there.

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... :-)

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!).

(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.)

Recommendations in decreasing order of effectiveness:
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.
2) don't block autoland UI in mozreview on open issues, just warn rather
than disabling it outright.
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.
4) provide fix/drop all remaining issues buttons on both the diff and
issue summary page
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.***
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.

> 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.
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.

> 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).
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.

Gijs


***
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.


_______________________________________________
firefox-dev mailing list
firef...@mozilla.org
https://mail.mozilla.org/listinfo/firefox-dev

Gregory Szorc

unread,
Jul 18, 2016, 4:56:51 PM7/18/16
to Gijs Kruitbosch, Firefox Dev, Gregory Szorc
On Thu, Jul 14, 2016 at 1:54 AM, Gijs Kruitbosch <gijskru...@gmail.com> wrote:
Hi,

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].

However, some issues with the reasoning here...

On 13/07/2016 22:10, Gregory Szorc wrote:
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).
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.

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.
 
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:

FWIW MozReview review requests are closed automatically when using autoland, making your dashboard usable.
 

1) push commits to mozreview. Get r+ with 5 nits to fix some days later
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.

Now I have two options:
a) push to fx-team
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. https://reviewboard.mozilla.org/r/62876/ ) has an Automation entry in the top navigation bar, but the diff view (e.g. https://reviewboard.mozilla.org/r/62876/diff/ ) does not, and so I usually spend at least some time looking for a button that's not there.

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... :-)


You are correct that 2nd workflow is pretty poor and that there are some UI warts around the "automation" entry in the navigation bar.

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.
 
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!).

(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.)

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.
 

Recommendations in decreasing order of effectiveness:
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.

I agree.
 
2) don't block autoland UI in mozreview on open issues, just warn rather than disabling it outright.

Please file a bug on this if there isn't already.
 
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.

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.
 
4) provide fix/drop all remaining issues buttons on both the diff and issue summary page
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.***

I think this is being addressed by the work to show comments inline.
 
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.

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.
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.

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).
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.

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.

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.
 


***
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

I wonder if this is a consequence of you being on a different continent from the server in California...
 

Bill McCloskey

unread,
Jul 18, 2016, 5:10:25 PM7/18/16
to Gregory Szorc, Firefox Dev, Gijs Kruitbosch
It's not. I work from the Mountain View office, I have a fast computer, and I plug into Ethernet. The spinners in MozReview are really bad. Even small patches go through a few seconds of spinners. For big ones, I might wait 30 seconds or more. Why can't the default view be assembled and cached on the server, with XHRs only used when you ask for more context? The main reason I prefer Splinter is that it's quick and simple. I think most people would be a lot happier if MozReview was looked like Splinter but with smarter diffing and the option to get more context.

-Bill

Nathan Froyd

unread,
Jul 18, 2016, 5:13:46 PM7/18/16
to Gregory Szorc, Firefox Dev, Gijs Kruitbosch
On Mon, Jul 18, 2016 at 4:56 PM, Gregory Szorc <g...@mozilla.com> wrote:
> On Thu, Jul 14, 2016 at 1:54 AM, Gijs Kruitbosch <gijskru...@gmail.com>
> wrote:
>> Just generally, it feels like mozreview is full of loading indicators.
>> Everywhere. Even things that should just be instantaneous
>
> I wonder if this is a consequence of you being on a different continent from
> the server in California...

I can assure you that even people on the same continent as California
experience "Loading..." indicators, even for very simple patches, and
it's very annoying. For instance, for:

https://reviewboard.mozilla.org/r/64944/diff/1#index_header

I can see a "Loading..." indicator flash at page load. I can also see
one when I click "Finish Review". I can't reproduce Gijs's other
complaints right now, as I've already done the review, but I can
certainly recall slowness in all the scenarios he has described.

-Nathan

Gijs Kruitbosch

unread,
Jul 20, 2016, 11:57:36 AM7/20/16
to Gregory Szorc, Firefox Dev
On 18/07/2016 21:56, Gregory Szorc wrote:
On Thu, Jul 14, 2016 at 1:54 AM, Gijs Kruitbosch <gijskru...@gmail.com> wrote:
On 13/07/2016 22:10, Gregory Szorc wrote:
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).
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.

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.
But a significant portion of some people's commits are tied to security bugs, and so those people are unhappy. I also wasn't exactly suggesting tyranny, just pointing out that it's not possible to use autoland without using mozreview, which in turn isn't possible for some bugs. Why not decouple autoland from mozreview? That would also make "land my amended patch" easier - it's basically the same as (1) in the list of suggestions. If "hg push -r . review --autoland" guaranteed to land my patch "soon" then I could use that for a security bug after I got review via splinter, but at the moment I can't even do that - I have to land it manually instead.

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:

FWIW MozReview review requests are closed automatically when using autoland, making your dashboard usable.
I'm well aware of this, my point was that if I don't use autoland they are not closed, and in too many cases autoland is the worse option, and the result is a useless dashboard.

Recommendations in decreasing order of effectiveness:
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.

I agree.
So is there a bug on file on making this more sane? Should I file one?

2) don't block autoland UI in mozreview on open issues, just warn rather than disabling it outright.

Please file a bug on this if there isn't already.


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.

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.
This was already on file. https://bugzilla.mozilla.org/show_bug.cgi?id=1232703 . I've pinged the assignee, david walsh, for updates, as it's been sitting there for the last 2 months, AFAICT.

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.
But I do want to use autoland. I use it pretty regularly. Sometimes it's a non-great option even though I 'want' to use it. That's not the same thing as not wanting to.

I also pointed out that inbound is inferior to fx-team both in terms of closure rates and in terms of stability. So practically speaking, "just substituting" one for the other leads to a deterioration in my effectiveness because the tools at my disposal just got worse.

I guess I'm more likely to substitute central instead, but that is also annoying because I can't push from it, and going from central to inbound in the same working directory invalidates my builds and means I then waste 20 minutes on a clobber - that or I have to do another "hg share" to have a dedicated "for pushing" working directory, and hop around between them whenever I push, which also feels like a chore.

Effectively, I'm having to pile workaround on workaround. I can do that, but I would prefer for the autoland story to get better sooner. How many people are working on that? Can we increase whatever that number is so it stops being non-great for a number of common usecases?

~ Gijs

Gijs Kruitbosch

unread,
Jul 20, 2016, 12:00:44 PM7/20/16
to Nathan Froyd, Gregory Szorc, Firefox Dev
On 18/07/2016 22:13, Nathan Froyd wrote:
> On Mon, Jul 18, 2016 at 4:56 PM, Gregory Szorc <g...@mozilla.com> wrote:
>> On Thu, Jul 14, 2016 at 1:54 AM, Gijs Kruitbosch <gijskru...@gmail.com>
>> wrote:
>>> Just generally, it feels like mozreview is full of loading indicators.
>>> Everywhere. Even things that should just be instantaneous
>> I wonder if this is a consequence of you being on a different continent from
>> the server in California...
> I can assure you that even people on the same continent as California
> experience "Loading..." indicators, even for very simple patches, and
> it's very annoying. For instance, for:
>
> https://reviewboard.mozilla.org/r/64944/diff/1#index_header
>
> I can see a "Loading..." indicator flash at page load. I can also see
> one when I click "Finish Review". I can't reproduce Gijs's other
> complaints right now, as I've already done the review, but I can
> certainly recall slowness in all the scenarios he has described.
>
> -Nathan

Nathan filed some bugs (thanks!), and I filed some more and a metabug:
https://bugzilla.mozilla.org/show_bug.cgi?id=1288133

~ Gijs
Reply all
Reply to author
Forward
0 new messages