<div dir="ltr">On Tue, Jan 26, 2016 at 12:06 PM, Mike Conley <span dir="ltr"><<a href="mailto:mconley@mozilla.com" target="_blank">mconley@mozilla.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">

<div style="font-family:"Helvetica Neue";font-size:14px">Last weekend, we closed out regression bug 1240559 just before beta 44 got shipped for release. The discovery was made in the last week of the beta, and the fix landed on Saturday, only a few days before the release. This bug showed textbook teamwork and decision making, and in some ways, it should be considered a success in terms of making sure we’re not shipping broken things.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">Despite top-notch teamwork resulting in a positive outcome, I suspect we should never have gotten into those conditions in the first place.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">It should go without saying that I am not interested in finger-pointing or casting blame. I think, however, that it’d be useful to examine what occurred with this bug to see if there are ways we can prevent similar occurrences in the future.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">Some context first:
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">Work has been underway to remotely host about:newtab. Some of that work has landed in tree for testing, and a decision was made to only expose that feature for testing in Aurora and Nightly.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">A patch was written in bug 1224950 to isolate the remote newtab code so that it only ever runs if we’re not in a RELEASE_BUILD. Part of that patch included a change to a moz.build to only package the remote newtab files if we’re not in a RELEASE_BUILD. The problem was that a file unrelated to the remote newtab project was included in that list of files not being packaged.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">This file, NewTabURL.jsm acts as an API for add-ons that want to override the URL used for newtabs. This value used to be stored in a pref, but was later hard-coded in with the ability to override at runtime in order to avoid newtab hijacking. NewTabURL.jsm is also special because outside of its sole unit test, it is not used from within any of the Firefox codebase. It’s relied on by add-ons, and add-ons only.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">The NewTabURL.jsm removal from the moz.build for MOZILLA_RELEASE builds file slipped through (my) review, and landed in mozilla-central. It was also uplifted to mozilla-aurora.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">Because the removal of NewTabURL.jsm was conditional upon MOZILLA_RELEASE, add-on developers who were testing on Nightly and Aurora builds did not notice anything strange. Everything seemed to work properly.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">It’s only when Aurora merged to Beta, and an add-on developer eventually tested the Beta, that the problem was discovered, and filed as bug 1240559. This was on January 18th, which is 8 days before the beta gets shipped to release.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">A workaround for add-on developers was posted in <a href="https://bugzilla.mozilla.org/show_bug.cgi?id=1240559#c10" target="_blank">https://bugzilla.mozilla.org/show_bug.cgi?id=1240559#c10</a>, and the bug was marked as a WONTFIX on the 21st. 
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">On the 22nd, an add-on developer reopened the bug, saying that the workaround wasn’t acceptable, as they wouldn’t have time to fix their add-on and get it approved before 44 went out the door.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">A patch was written, and then the decision was made to land the fix directly onto release. The add-on author confirmed the fix, and the bug was closed out. This is what we ended up shipping.
</div><div style="font-family:"Helvetica Neue";font-size:14px"><br></div><div style="font-family:"Helvetica Neue";font-size:14px">Tentative recommendations to avoid such last minute things in the future:
</div><ol style="font-family:"Helvetica Neue";font-size:14px"><li>Patch authors and reviewers should avoid, if possible, allowing MOZILLA_RELEASE decision-making in patches. They’re effectively chopping off 12 weeks of testing for particular code paths / conditions. I understand there are things that we want to halt at the beta level of the trains *cough e10s cough*, but the logic for disabling the feature should be as isolated as possible, and should probably not change the files being packaged.</li><li>Gecko is an ever-changing API surface that add-ons rely on. Early notice of changes is good, but sometimes addon-compat flags aren’t set and things slip through the cracks. It might be worth messaging to add-on authors that hgweb allows developers to “subscribe” to changes for a particular file through an Atom feed. Example: <a href="http://hg.mozilla.org/mozilla-central/atom-log/tip/browser/components/newtab/moz.build" target="_blank">http://hg.mozilla.org/mozilla-central/atom-log/tip/browser/components/newtab/moz.build</a> .We might also want to consider somehow making it simpler to watch files for changes across the central, aurora and beta branches. I figure this is as “simple” as writing a specialized Atom feed reader.</li></ol></div></blockquote><div><br></div><div>Building a "file watching service" has been on my radar for a long time. We want this for MozReview so people can "subscribe" to proposed changes to files they care about so they can automatically be added as a reviewer or at least be CCd on the review/bug. (Bugzilla component watching is insufficient because patches may not be in the followed component. Commits in the review repositories are closer to the action than bugs and following them is less error prone.)<br><br></div><div>I also want the "file watching service" so people can get notification after things land. Maybe you don't want to actively take part in review but you do want to casually audit things that land. I know Dave Townsend has built something like this for himself and there is a market for it. If we're building a review subscription service, I think we can build this feature easily as well.<br><br></div><div>I have a deliverable for this quarter that will produce some infrastructure to make building a "file watching service" a bit easier. Unfortunately, there are no definite plans to build a file watching service at this time.<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><ol style="font-family:"Helvetica Neue";font-size:14px"><li>We should be encouraging add-on authors to test their add-ons on the Beta channel, as well as Nightly and Aurora (or perhaps in place of).</li><li>Experimental components and modules should not be mixed with shipping components and modules in moz.build files. Or, if they’re going to be mixed, they should ship in the builds, even if they’re not being used.</li><li>When regressions show up late in the game on a beta, at the very least, a Firefox peer should be needinfo’d on the bug to give their input.</li><li>When the regressing bug is found, the bug for the regression should be marked blocking the regressing bug ASAP, in order to increase the likelihood of the right people noticing the regression.</li><li>We should move NewTabURL.jsm under browser/modules to keep it isolated from the experimentation currently underway in browser/components/newtab.</li></ol><div style="font-family:"Helvetica Neue";font-size:14px">Is there anything on this list of recommendations that’s controversial? Or anything that I’m missing? Is any of the above worth taking action on, and who should be responsible for taking action on them?
</div></div>
<br>_______________________________________________<br>
firefox-dev mailing list<br>
<a href="mailto:firefox-dev@mozilla.org" target="_blank">firefox-dev@mozilla.org</a><br>
<a href="https://mail.mozilla.org/listinfo/firefox-dev" rel="noreferrer" target="_blank">https://mail.mozilla.org/listinfo/firefox-dev</a><br>
<br></blockquote></div><br></div></div>