Re: The future of commit access policy for core Firefox

On 03/13/2017 01:23 PM, Eric Shepherd (Sheppy) wrote:
>> On Mar 13, 2017, at 3:48 PM, Justin Dolske <[email protected]> wrote:
>>
>> Specifically, here: if a reviewer has already decided that a patch is "r+ with fixes", it's unlikely that the followup patch is going to get a vigorous, detailed review. Especially if the process is perceived as pointless overhead, causing delays, and 99.9999% of the time the patch author is not trying to sneak in malicious code.
>>
>> So a simple "every patch must be reviewed" requirement which doesn't address that in some way isn't really going to change much from the status quo -- you'd get "reviews", but only as a paperwork formality.
>
> That brings to mind this question, for me: does it make more sense to either shoot for a slightly improved review process but find and implement practices that have automated security verifications being performed on the code when it’s submitted for review, even before the reviewer sees it? Before you jump, yes, I know these tools aren’t able to catch all the varieties of issues that exist, but it would certainly help. (Personally, this is an area we could apply some resources to, if only by donation of funds).
>
> Anyway, once the automated security scans are complete and validated, only then would the reviewer actually do their review. They’d still have to do security related checks, but they’d have backup, and hopefully the really tiny changes, such as "nit” fixes would have a pretty low risk factor given the automated analysis. If it gets sent back for changes — of any kind — it goes through security scans again.
>
> I may or may not have made sense, but hopefully I did. Just throwing thoughts out there.

Lest people find this unrealistic, I would like to point out that within 
the JS engine at least, we sort of already have this via fuzzing. It's 
not an analysis of a specific change, it's the analysis of the overall 
state after a set of changes, with mostly automated bisection when 
something is found. I believe it provides the sorts of benefit sheppy is 
describing, but in a retroactive way -- reviewers don't have the results 
in hand at the time of review, but at least they can depend on one class 
of problems being caught before too long.

Obviously, it relies heavily on how fuzzable a component is.

_______________________________________________
dev-planning mailing list
[email protected]
https://lists.mozilla.org/listinfo/dev-planning