Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2705153006:
[DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files.
|
Created:
3 years, 10 months ago by ymzhang1 Modified:
3 years, 4 months ago CC:
chfremer+watch_chromium.org, chromium-reviews, darin-cc_chromium.org, feature-media-reviews_chromium.org, jam, mcasas+watch+vc_chromium.org, miu+watch_chromium.org, mlamouri+watch-content_chromium.org, piman+watch_chromium.org, posciak+watch_chromium.org, xjz+watch_chromium.org, magjed_chromium, wuchengli Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files.
We are adding COMPONENT/TEAM information into OWNERS file. Please help us to
verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your
OWNERS files. Thanks.
Proposal to add TEAM/COMPONENT information into OWNERS files
http://bit.ly/add-team-component-proposal
Proposal about how to get suggested component for directory.
http://bit.ly/directory-mapping-proposal
TEAM-COMPONENT mapping
http://bit.ly/component-team-mapping
Additional Information:
Component lists
https://bugs.chromium.org/p/chromium/adminComponents
BUG=679905
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Patch Set 1 #
Total comments: 7
Patch Set 2 : update component #
Total comments: 6
Patch Set 3 : update components #Patch Set 4 : remove wrong file #
Total comments: 2
Patch Set 5 : update component #Patch Set 6 : add owner for webrtc #Patch Set 7 : add emircan in content/renderer/media/gpu/OWNERS #
Messages
Total messages: 33 (13 generated)
Description was changed from ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 ========== to ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
ymzhang@chromium.org changed reviewers: + posciak@chromium.org
Hello, We are adding COMPONENT/TEAM information into OWNERS file. Would you mind helping us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files? Thank you very much!
Description was changed from ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
posciak@chromium.org changed reviewers: + dalecurtis@chromium.org
https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>GPU The component spanning all should probably be Internals>Media>Video. However, for example for CrOS classes/files, OS>Kernel>Video would be preferred. Given that (per my understanding) we can have only one COMPONENT per OWNERS, we'd have to either split media/gpu into subdirectories, or manually triage from Internals>Media>Video. What would be preferable? https://codereview.chromium.org/2705153006/diff/1/third_party/libva/OWNERS File third_party/libva/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/third_party/libva/OWNERS#ne... third_party/libva/OWNERS:3: # COMPONENT: Internals>Network>Library OS>Kernel>Video please https://codereview.chromium.org/2705153006/diff/1/third_party/v4l-utils/OWNERS File third_party/v4l-utils/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/third_party/v4l-utils/OWNER... third_party/v4l-utils/OWNERS:4: # COMPONENT: Internals>Network>Library OS>Kernel>Video please
posciak@chromium.org changed reviewers: + wuchengli@chromium.org
Description was changed from ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>GPU On 2017/03/24 01:39:05, Pawel Osciak wrote: > The component spanning all should probably be Internals>Media>Video. However, > for example for CrOS classes/files, OS>Kernel>Video would be preferred. > > Given that (per my understanding) we can have only one COMPONENT per OWNERS, > we'd have to either split media/gpu into subdirectories, or manually triage from > Internals>Media>Video. > > What would be preferable? Thanks for the comments! Yes, your understanding is correct. Currently, I updated to Internals>Media>Video. Spliting media/gpu into subdirectories and adding proper components for them would be more helpful. It is actually a better choice if it doesn't break current codes. https://codereview.chromium.org/2705153006/diff/1/third_party/libva/OWNERS File third_party/libva/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/third_party/libva/OWNERS#ne... third_party/libva/OWNERS:3: # COMPONENT: Internals>Network>Library On 2017/03/24 01:39:05, Pawel Osciak wrote: > OS>Kernel>Video please Done. https://codereview.chromium.org/2705153006/diff/1/third_party/v4l-utils/OWNERS File third_party/v4l-utils/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/third_party/v4l-utils/OWNER... third_party/v4l-utils/OWNERS:4: # COMPONENT: Internals>Network>Library On 2017/03/24 01:39:05, Pawel Osciak wrote: > OS>Kernel>Video please Done.
https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... File content/renderer/media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... content/renderer/media/gpu/OWNERS:4: # COMPONENT: Internals>GPU Internals>Media>Hardware? https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>Media>Video Internals>Media>Hardware (though we should rename this label to Internals>Media>GPU)
On 2017/03/24 17:22:31, DaleCurtis wrote: > https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... > File content/renderer/media/gpu/OWNERS (right): > > https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... > content/renderer/media/gpu/OWNERS:4: # COMPONENT: Internals>GPU > Internals>Media>Hardware? > > https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS > File media/gpu/OWNERS (right): > > https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS#newcode15 > media/gpu/OWNERS:15: # COMPONENT: Internals>Media>Video > Internals>Media>Hardware (though we should rename this label to > Internals>Media>GPU) dalecurtis@: what would be your view on the discussion in media/gpu/OWNERS on PS1? Thanks!
https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/1/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>GPU On 2017/03/24 at 16:43:39, ymzhang1 wrote: > On 2017/03/24 01:39:05, Pawel Osciak wrote: > > The component spanning all should probably be Internals>Media>Video. However, > > for example for CrOS classes/files, OS>Kernel>Video would be preferred. > > > > Given that (per my understanding) we can have only one COMPONENT per OWNERS, > > we'd have to either split media/gpu into subdirectories, or manually triage from > > Internals>Media>Video. > > > > What would be preferable? > > Thanks for the comments! Yes, your understanding is correct. Currently, I updated to Internals>Media>Video. > > Spliting media/gpu into subdirectories and adding proper components for them would be more helpful. It is actually a better choice if it doesn't break current codes. Adding platform specific subdirs sgtm, we already do this for media/audio/{mac,win,linux,android,cras}
https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>Media>Video On 2017/03/24 17:22:31, DaleCurtis wrote: > Internals>Media>Hardware (though we should rename this label to > Internals>Media>GPU) Hi dalecurtis@ and posciak@, Thanks for the comments! Could we have an agreement on one primary component for this directory?
https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>Media>Video On 2017/03/27 at 21:32:05, ymzhang1 wrote: > On 2017/03/24 17:22:31, DaleCurtis wrote: > > Internals>Media>Hardware (though we should rename this label to > > Internals>Media>GPU) > > Hi dalecurtis@ and posciak@, > > Thanks for the comments! Could we have an agreement on one primary component for this directory? Internals>Media>Hardware should be the component for now. I'll see about renaming it to Internals>Media>GPU later. Once we breakout OS specific impls, we can override the subdir with cros.
Patchset #3 (id:40001) has been deleted
https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... File content/renderer/media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/content/renderer/media/... content/renderer/media/gpu/OWNERS:4: # COMPONENT: Internals>GPU On 2017/03/24 17:22:30, DaleCurtis wrote: > Internals>Media>Hardware? Done. https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS File media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/20001/media/gpu/OWNERS#newcode15 media/gpu/OWNERS:15: # COMPONENT: Internals>Media>Video On 2017/03/27 21:35:08, DaleCurtis wrote: > On 2017/03/27 at 21:32:05, ymzhang1 wrote: > > On 2017/03/24 17:22:31, DaleCurtis wrote: > > > Internals>Media>Hardware (though we should rename this label to > > > Internals>Media>GPU) > > > > Hi dalecurtis@ and posciak@, > > > > Thanks for the comments! Could we have an agreement on one primary component > for this directory? > > Internals>Media>Hardware should be the component for now. I'll see about > renaming it to Internals>Media>GPU later. Once we breakout OS specific impls, we > can override the subdir with cros. Cool. Thx!
media/ lgtm
https://codereview.chromium.org/2705153006/diff/80001/content/renderer/media/... File content/renderer/media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/80001/content/renderer/media/... content/renderer/media/gpu/OWNERS:4: # COMPONENT: Internals>Media>Hardware Actually I think this one is wrong; the only things left in here are WebRTC code. So someone from WebRTC should be the owners here. posciak,wuchengli are remnants from before we had media/gpu
https://codereview.chromium.org/2705153006/diff/80001/content/renderer/media/... File content/renderer/media/gpu/OWNERS (right): https://codereview.chromium.org/2705153006/diff/80001/content/renderer/media/... content/renderer/media/gpu/OWNERS:4: # COMPONENT: Internals>Media>Hardware On 2017/03/27 22:06:25, DaleCurtis wrote: > Actually I think this one is wrong; the only things left in here are WebRTC > code. So someone from WebRTC should be the owners here. posciak,wuchengli are > remnants from before we had media/gpu We do have some WebRTC related components on list(https://bugs.chromium.org/p/chromium/adminComponents). e.g. Internals>WebRTC, Blink>WebRTC, etc. Maybe one of these components are more proper?
Internals>WebRTC seems correct, we should replace the OWNERS in that file with folk from WebRTC too. Want to add one of the OWNERS for that component?
ymzhang@chromium.org changed reviewers: + pthatcher@chromium.org
Hi pthatcher@, Could you please help to review component in content/renderer/media/gpu/OWNERS? Meanwhile, we would like to add one owner for WebRTC in this OWNERS files. Would you mind giving some suggestion? Thanks! Best, Yangmuzi
I would suggest for tommi@ to take a look and make a suggestion. I'm involved with WebRTC, but not so much with GPU stuff. I think tommi@ would have enough context to make the right suggestion or point you to someone that can. On 2017/03/27 22:38:02, ymzhang1 wrote: > Hi pthatcher@, > > Could you please help to review component in content/renderer/media/gpu/OWNERS? > > Meanwhile, we would like to add one owner for WebRTC in this OWNERS files. Would > you mind giving some suggestion? > > Thanks! > > Best, > Yangmuzi
Description was changed from ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ========== to ========== [DirectoryOwnership] Add TEAM/COMPONENT into OWNERS files. We are adding COMPONENT/TEAM information into OWNERS file. Please help us to verify the added TEAM/COMPONENT or suggest the correct TEAM/COMPONENT in your OWNERS files. Thanks. Proposal to add TEAM/COMPONENT information into OWNERS files http://bit.ly/add-team-component-proposal Proposal about how to get suggested component for directory. http://bit.ly/directory-mapping-proposal TEAM-COMPONENT mapping http://bit.ly/component-team-mapping Additional Information: Component lists https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel ==========
pthatcher@chromium.org changed reviewers: + tommi@chromium.org
lgtm. For webrtc, please add magjed@ (cc-ed)
ymzhang@chromium.org changed reviewers: + magjed@chromium.org
Add magjed@ in content/renderer/media/gpu/OWNERS.
On 2017/04/22 22:41:20, ymzhang1 wrote:
> Add magjed@ in content/renderer/media/gpu/OWNERS.
I don't think magjed@ should be in content/renderer/media/gpu/OWNERS
but emircan@ should be there.
Rationale:
git shortlog -s -n content/renderer/media/gpu
12 emircan
4 wuchengli
3 braveyao
3 magjed
2 watk
1 ilnik
1 j.isorce
1 jbriance
1 jem456.vasishta
1 jyasskin
1 asvitkine
1 nisse
1 pbos
1 pkasting
1 ricea
1 sergeyu
1 skvlad
1 skyostil
1 thestig
1 leon.han
1 avi
1 dalecurtis
1 gab
1 hta
he's also not very active in the counterpart media/gpu:
git shortlog -s -n media/gpu/
48 jbauman
38 watk
30 liberato
30 emircan
24 wuchengli
20 dalecurtis
20 sandersd
18 hubbe
15 kcwu
12 posciak
10 henryhsu
10 fdoray
9 mcasas
8 markdittmer
8 xhwang
7 gab
6 chfremer
6 owenlin
6 kylechar
6 dcastagna
6 jrummell
5 thakis
5 vmpstr
4 johnylin
4 julien.isorce
4 hywu
3 brucedawson
3 ananta
3 braveyao
3 fsamuel
3 guidou
3 jam
3 jbriance
3 piman
3 robliao
3 shenghao
3 vince.h
2 jcliang
2 asvitkine
2 ccameron
2 dcheng
2 lionel.g.landwerlin
2 tobiasjs
2 rockot
2 lizeb
2 penghuang
2 avi
2 kbr
2 dpranke
2 battre
2 geofflang
2 servolk
2 jem456.vasishta
2 brettw
2 sunnyps
1 ricea
1 rnephew
1 John Bauman
1 rsesek
1 sakal
1 sanfin
1 scheib
On 2017/04/24 02:10:15, mcasas wrote: > On 2017/04/22 22:41:20, ymzhang1 wrote: > > Add magjed@ in content/renderer/media/gpu/OWNERS. > > I don't think magjed@ should be in content/renderer/media/gpu/OWNERS > but emircan@ should be there. > > Rationale: > > git shortlog -s -n content/renderer/media/gpu > 12 emircan > 4 wuchengli > 3 braveyao > 3 magjed > 2 watk > 1 ilnik > 1 j.isorce > 1 jbriance > 1 jem456.vasishta > 1 jyasskin > 1 asvitkine > 1 nisse > 1 pbos > 1 pkasting > 1 ricea > 1 sergeyu > 1 skvlad > 1 skyostil > 1 thestig > 1 leon.han > 1 avi > 1 dalecurtis > 1 gab > 1 hta > > he's also not very active in the counterpart media/gpu: > git shortlog -s -n media/gpu/ > 48 jbauman > 38 watk > 30 liberato > 30 emircan > 24 wuchengli > 20 dalecurtis > 20 sandersd > 18 hubbe > 15 kcwu > 12 posciak > 10 henryhsu > 10 fdoray > 9 mcasas > 8 markdittmer > 8 xhwang > 7 gab > 6 chfremer > 6 owenlin > 6 kylechar > 6 dcastagna > 6 jrummell > 5 thakis > 5 vmpstr > 4 johnylin > 4 julien.isorce > 4 hywu > 3 brucedawson > 3 ananta > 3 braveyao > 3 fsamuel > 3 guidou > 3 jam > 3 jbriance > 3 piman > 3 robliao > 3 shenghao > 3 vince.h > 2 jcliang > 2 asvitkine > 2 ccameron > 2 dcheng > 2 lionel.g.landwerlin > 2 tobiasjs > 2 rockot > 2 lizeb > 2 penghuang > 2 avi > 2 kbr > 2 dpranke > 2 battre > 2 geofflang > 2 servolk > 2 jem456.vasishta > 2 brettw > 2 sunnyps > 1 ricea > 1 rnephew > 1 John Bauman > 1 rsesek > 1 sakal > 1 sanfin > 1 scheib Makes sense to me assuming Emircan is OK with it :)
ymzhang@chromium.org changed reviewers: + emircan@chromium.org - magjed@chromium.org
Add emircan in content/renderer/media/gpu/OWNERS.
wuchengli@chromium.org changed reviewers: - wuchengli@chromium.org |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
