net/socket/OWNERS - Issue 1898603002: Move Network Quality Estimator files to //net/nqe

Side by Side Diff: net/socket/OWNERS

Issue 1898603002: Move Network Quality Estimator files to //net/nqe (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: Created 4 years, 8 months 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
(Empty)
	1 per-file socket_performance_watcher*=bengr@chromium.org
	Ryan Sleevi 2016/04/27 20:51:08 I would prefer not to introduce a per-file OWNERS I would prefer not to introduce a per-file OWNERS here. While I realize Ben is unquestionably the person most likely to know how this all relates, ideally, we would have more members of the team who know how to review this code. Ben, do you feel strongly about this? Did I miss a concern? tbansal1 2016/04/27 20:59:49 Ben can add more but probably this is my bad. I as Show quoted text On 2016/04/27 20:51:08, Ryan Sleevi wrote: > I would prefer not to introduce a per-file OWNERS here. While I realize Ben is > unquestionably the person most likely to know how this all relates, ideally, we > would have more members of the team who know how to review this code. > > Ben, do you feel strongly about this? Did I miss a concern? Ben can add more but probably this is my bad. I assumed the OWNERS from the parent folder would also apply to files in this folder too (because there is no set noparent here). Is that not the case here? tbansal1 2016/04/27 21:24:31 According to https://chromium.googlesource.com/chr Show quoted text On 2016/04/27 20:59:49, tbansal1 wrote: > On 2016/04/27 20:51:08, Ryan Sleevi wrote: > > I would prefer not to introduce a per-file OWNERS here. While I realize Ben is > > unquestionably the person most likely to know how this all relates, ideally, > we > > would have more members of the team who know how to review this code. > > > > Ben, do you feel strongly about this? Did I miss a concern? > > Ben can add more but probably this is my bad. I assumed the OWNERS from the > parent folder would also apply to files in this folder too (because there is no > set noparent here). Is that not the case here? According to https://chromium.googlesource.com/chromium/tools/depot_tools/+/master/owners.py, if there is no "set noparent" directive, OWNERS from parent folder (//net/OWNERS) are also owners of all the files (including the files that have per-file owners) in this folder. Ryan, does that answer the concern? Ryan Sleevi 2016/04/28 16:14:44 Well, not quite, because I still am not terribly c Show quoted text On 2016/04/27 21:24:31, tbansal1 wrote: > Ryan, does that answer the concern? Well, not quite, because I still am not terribly comfortable about per-file OWNERS, so I wanted to confirm with Ben that he still wanted this. I'm not going to object, but I'd like to make sure this is something intentional with Ben. Definitely, the relationship with with the rest of socket APIs is important to consider, so does it make sense for socket owners to do reviewing? tbansal1 2016/04/28 20:06:45 Done. I have removed //net/socket/OWNERS. Show quoted text On 2016/04/28 16:14:44, Ryan Sleevi wrote: > On 2016/04/27 21:24:31, tbansal1 wrote: > > Ryan, does that answer the concern? > > Well, not quite, because I still am not terribly comfortable about per-file > OWNERS, so I wanted to confirm with Ben that he still wanted this. I'm not going > to object, but I'd like to make sure this is something intentional with Ben. > Definitely, the relationship with with the rest of socket APIs is important to > consider, so does it make sense for socket owners to do reviewing? Done. I have removed //net/socket/OWNERS.
OLD	NEW

« no previous file with comments | « net/quic/quic_stream_factory.cc ('k') | net/socket/client_socket_factory.h » ('j') | no next file with comments »