Intent to Implement System Add-on: SHIELD/Normandy

Gijs Kruitbosch gijskruitbosch at gmail.com
Tue Sep 27 15:08:18 UTC 2016


On 27/09/2016 02:10, Michael Kelly wrote:
> On 9/26/16 2:05 PM, Gijs Kruitbosch wrote:
>> Hi,
>>
>>  From this, I'm assuming Firefox peers/owners will be asked to review the
>> add-on code before landing it in m-c. Can you confirm if that's correct?
>> Stating the obvious and related point: it might be worth giving them a
>> heads up (if that hasn't already happened) if you're planning to dump a
>> big load of code in their queue...
> Yep! Likely candidates who have helped during the implementation include
> rhelmer, MattN, and gfritzsche. We're going to pick one ASAP and give
> them a heads up / ask when they will have time for the review.
Great.
>> Secondly... are the actions / server-side things going to be subject to
>> Firefox reviews, given that they directly affect the user experience of
>> the product in ways that normally require such reviews? There's some
>> unfortunate history in that area that I would really like to avoid
>> repetitions of.
> The plan is currently that they be subject to reviews, but I'm not sure
> if they match what you mean by "Firefox reviews". I'm not terribly
> familiar with how Firefox reviews work (and when I last asked I got
> mixed answers).
You stick code and/or a patch somewhere people in this list[0] can see 
it, you pick one or more of them, and they either approve it or tell you 
what to change before it gets approved, lands, and becomes part of 
Firefox-the-product. I am curious what you mean by "mixed answers".
[0] https://wiki.mozilla.org/Modules/Firefox

> The doc page at
> http://normandy.readthedocs.io/en/latest/dev/concepts.html explains in a
> bit more detail, but in terms of technical review measures:
> <snip>
> - Changes to the JS code run within the sandbox will be reviewed within
> our team, as that code is part of the Normandy service codebase and is
> reviewed in the same manner (On Github in a pull request).
OK, so for this one the answer is "no". This concerns me because that 
code ultimately affects how Firefox-the-product behaves (outside of what 
webpages can do) and normally such changes are subject to Firefox's 
review policy. This is the case for other go-faster add-ons (e.g. I have 
reviewed a number of changes to the pocket "go faster" add-on in the 
last few weeks). When this was not the case for self-repair code, that 
burned us.

Followup question: does this code in any way follow a train model (ie 
get tested against Firefox Nightly users first) ? The self-repair code 
didn't and just ran against 10% of release users to test it, which, 
again, burned us in the past.

Related, why can't the experiments people "want" to run be run against 
Nightly users first to test the viability of the system add-on and the 
rest of the platforms involved with running these 
recipes/experiments/snippets? Why the need to immediately uplift to Aurora?

> - The configuration passed to the JS code will be peer-reviewed so that
> at least 2 people with access to edit recipes on the server agree to the
> changes.
What's required to "back out" or "disable" or "turn off" a recipe or 
this JS code / configuration? The docs say enabling a recipe takes 2 
people, so I'm assuming we're talking about the same thing. How many 
people get access to the "turn it off" switch and where are they located 
(both team/org-wise and timezone/location-wise) ? Again, I don't want to 
end up with a situation where something gets approved and then needs 
changing and nobody is available to do that.

Related: are there controls to stop us from deploying stuff to release 
late PDT (or on Friday or whatever) so that the world burns while the US 
and Europe are asleep or on holiday?

~ Gijs



More information about the firefox-dev mailing list