Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(36)

Issue 2582593002: Adds set noparent to webkit/OWNERS (Closed)

Created:
4 years ago by sky
Modified:
4 years ago
CC:
abarth-chromium, blink-reviews, chromium-reviews
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

Adds set noparent to webkit/OWNERS Without this owners are inherited from third_party/OWNERS, which isn't what we want here. R=dglazkov@chromium.org, jochen@chromium.org BUG=none

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -0 lines) Patch
M third_party/WebKit/OWNERS View 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
sky
4 years ago (2016-12-15 17:07:24 UTC) #1
jochen (gone - plz use gerrit)
we used to have that but Nico didn't like it for some reason I forgot
4 years ago (2016-12-15 18:36:02 UTC) #3
Nico
On 2016/12/15 18:36:02, jochen wrote: > we used to have that but Nico didn't like ...
4 years ago (2016-12-15 18:42:22 UTC) #4
sky
4 years ago (2016-12-15 18:50:11 UTC) #5
On 2016/12/15 18:42:22, Nico (out Thu Dec 15) wrote:
> On 2016/12/15 18:36:02, jochen wrote:
> > we used to have that but Nico didn't like it for some reason I forgot
> 
> I don't think we should use noparent more than absolutely necessary, and
> third_party owners should all know that they shouldn't approve changes in
> WebKit. Did the absence of this cause any issues? (Eventually this will move
to
> blink/, rendering the issue moot)

John echoed that 'noparent' is an anit-pattern, so I'll close this. His
reasoning is that with 'noparent' you can't get a top-level owner to review
mechanical changes that happen to touch a lot of files. This is certainly true,
but to counter that in large mechanical changes more often than not a top-level
owner is going to rubber stamp, so that you could just as well TBR the change.

As to the specific case. It's not that I approved a change to webkit (I wouldn't
do that), it's that I was put on a review with a bunch of reviewers that
happened to touch webkit and didn't realize the author of the patch didn't have
a webkit reviewer. So, my LGTM for chrome also covered the webkit change. If we
had a 'noparent' that would not have happened.
https://codereview.chromium.org/2545693002/ is the change.

Powered by Google App Engine
This is Rietveld 408576698