tools/metrics/OWNERS - Issue 1437683006: Give tools/ a real OWNERS file.

Side by Side Diff

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Keyboard Shortcuts

	File
u :	up to issue
j / k :	jump to file after / before current file
J / K :	jump to next file with a comment after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Up> / <Down> :	next / previous line

	Issue
u :	up to list of issues
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue

Side by Side Diff: tools/metrics/OWNERS

Issue 1437683006: Give tools/ a real OWNERS file. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Give tools/ a real OWNERS file. Created 5 years, 1 month ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
1 set noparent
Alexei Svitkine (slow) 2015/11/12 21:10:26 I think removing this is dangerous on tools/metric I think removing this is dangerous on tools/metrics. Now, if you have an LGTM from a src/OWNERS (e.g. because you needed a stamp for chrome/ or something), you might end up submitting without a metrics reviewer looking at your histograms.xml changes. You can do this accidentally (i.e. because the tools told you had complete OWNERS coverage). I don't think that's a good practice. Can we still have setnoparent at least for the XML files? Otherwise, I feel it's too easy to accidentally land those changes without a proper metrics review. Nico 2015/11/12 21:19:15 This is true for all OWNERS files though, right? Y Show quoted text On 2015/11/12 21:10:26, Alexei Svitkine (slow) wrote: > I think removing this is dangerous on tools/metrics. > > Now, if you have an LGTM from a src/OWNERS (e.g. because you needed a stamp for > chrome/ or something), you might end up submitting without a metrics reviewer > looking at your histograms.xml changes. You can do this accidentally (i.e. > because the tools told you had complete OWNERS coverage). This is true for all OWNERS files though, right? You have to trust that people in OWNERS files above you don't abuse their powers. Show quoted text > I don't think that's a good practice. Can we still have setnoparent at least for > the XML files? Otherwise, I feel it's too easy to accidentally land those > changes without a proper metrics review. Having `noparent` is bad practice too, in general as it makes global refactorings harder and whatnot. But if per-file noparent is possible, having it for the xml files in this directory would be fine with me (they do seem delicate, and per-file noparent sets a slightly worse precedent than a full noparent). But maybe we could wait a week or two and see if this a real problem first?
2

3 asvitkine@chromium.org	1 asvitkine@chromium.org

4 isherman@chromium.org	2 isherman@chromium.org

5 jar@chromium.org	3 jar@chromium.org

6 jwd@chromium.org	4 jwd@chromium.org

7 mpearson@chromium.org	5 mpearson@chromium.org

8 rkaplow@chromium.org	6 rkaplow@chromium.org

OLD	NEW

« tools/OWNERS ('K') | « tools/memory/OWNERS ('k') | tools/perf/OWNERS » ('j') | no next file with comments »