|
|
Chromium Code Reviews
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:
List of components
https://bugs.chromium.org/p/chromium/adminComponents
BUG=679905
Review-Url: https://codereview.chromium.org/2683853002
Cr-Commit-Position: refs/heads/master@{#452558}
Committed: https://chromium.googlesource.com/chromium/src/+/fecb2dd6f9168e9a6f381a1ea9565aa27ffad56f
Patch Set 1 #
Total comments: 4
Patch Set 2 : remove improper component #
Total comments: 2
Patch Set 3 : remove wrong component #
Messages
Total messages: 18 (9 generated)
ymzhang@chromium.org changed reviewers: + blundell@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 ========== presubmit error Add TEAM/COMPONENT BUG=679905 ========== to ========== presubmit error Add TEAM/COMPONENT BUG=679905 ==========
Description was changed from ========== presubmit error Add TEAM/COMPONENT BUG=679905 ========== to ========== BUG=679905 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 https://docs.google.com/document/d/1jty6UsFMW9-SYgpQC-ztEc3lltziOQBBArMkROCST... Proposal about how to get suggested component for directory. https://docs.google.com/document/d/1G0UD01aNXdzPsJO-gTLEQiKkDV8XU2XBbi_Gm4g9J... TEAM-COMPONENT mapping https://docs.google.com/spreadsheets/d/19JEFMvsxD3eThyGiJRqAjcpx362LHUDdVzICA... ==========
Description was changed from ========== BUG=679905 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 https://docs.google.com/document/d/1jty6UsFMW9-SYgpQC-ztEc3lltziOQBBArMkROCST... Proposal about how to get suggested component for directory. https://docs.google.com/document/d/1G0UD01aNXdzPsJO-gTLEQiKkDV8XU2XBbi_Gm4g9J... TEAM-COMPONENT mapping https://docs.google.com/spreadsheets/d/19JEFMvsxD3eThyGiJRqAjcpx362LHUDdVzICA... ========== 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: List of components https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 ==========
I don't have great suggestions from looking through the list of components, so added a description of what the code in question does. https://codereview.chromium.org/2683853002/diff/1/device/time_zone_monitor/OW... File device/time_zone_monitor/OWNERS (right): https://codereview.chromium.org/2683853002/diff/1/device/time_zone_monitor/OW... device/time_zone_monitor/OWNERS:4: # COMPONENT: UI>Notifications This doesn't seem like a fit. This code monitors for time zone changes and sends updates to registered clients on those changes. https://codereview.chromium.org/2683853002/diff/1/device/wake_lock/OWNERS File device/wake_lock/OWNERS (right): https://codereview.chromium.org/2683853002/diff/1/device/wake_lock/OWNERS#new... device/wake_lock/OWNERS:3: # COMPONENT: UI>Notifications This doesn't seem like a fit. This code allows clients to request that the device/screen be blocked from going to sleep.
https://codereview.chromium.org/2683853002/diff/1/device/time_zone_monitor/OW... File device/time_zone_monitor/OWNERS (right): https://codereview.chromium.org/2683853002/diff/1/device/time_zone_monitor/OW... device/time_zone_monitor/OWNERS:4: # COMPONENT: UI>Notifications On 2017/02/20 12:23:53, blundell wrote: > This doesn't seem like a fit. This code monitors for time zone changes and sends > updates to registered clients on those changes. Thanks! I'll remove this line. https://codereview.chromium.org/2683853002/diff/1/device/wake_lock/OWNERS File device/wake_lock/OWNERS (right): https://codereview.chromium.org/2683853002/diff/1/device/wake_lock/OWNERS#new... device/wake_lock/OWNERS:3: # COMPONENT: UI>Notifications On 2017/02/20 12:23:53, blundell wrote: > This doesn't seem like a fit. This code allows clients to request that the > device/screen be blocked from going to sleep. Thanks! Maybe it's better not to have any component information here.
rdsmith@: FYI for the cookie_config component lgtm with removal of line in //third_party/WebKit https://codereview.chromium.org/2683853002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/time_zone_monitor/OWNERS (right): https://codereview.chromium.org/2683853002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/time_zone_monitor/OWNERS:4: # COMPONENT: UI>Notifications sorry, this is the same as the other time_zone_monitor comment.
ymzhang@chromium.org changed reviewers: + rdsmith@chromium.org
Hi rdsmith@, Would you mind helping to review component in components/cookie_config/OWNERS? Thanks! - Yangmuzi https://codereview.chromium.org/2683853002/diff/20001/third_party/WebKit/Sour... File third_party/WebKit/Source/modules/time_zone_monitor/OWNERS (right): https://codereview.chromium.org/2683853002/diff/20001/third_party/WebKit/Sour... third_party/WebKit/Source/modules/time_zone_monitor/OWNERS:4: # COMPONENT: UI>Notifications On 2017/02/22 10:06:58, blundell wrote: > sorry, this is the same as the other time_zone_monitor comment. Done.
I don't know anything about components/cookie_config, but I'm happy with the use of Internals>Network>Cookies if the owners of that directory are. LGTM.
The CQ bit was checked by ymzhang@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from blundell@chromium.org Link to the patchset: https://codereview.chromium.org/2683853002/#ps40001 (title: "remove wrong component")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 40001, "attempt_start_ts": 1487873042367110,
"parent_rev": "60dfd97938a210a44495710ea75cb100c48cf8fd", "commit_rev":
"fecb2dd6f9168e9a6f381a1ea9565aa27ffad56f"}
Message was sent while issue was closed.
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: List of components 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: List of components https://bugs.chromium.org/p/chromium/adminComponents BUG=679905 Review-Url: https://codereview.chromium.org/2683853002 Cr-Commit-Position: refs/heads/master@{#452558} Committed: https://chromium.googlesource.com/chromium/src/+/fecb2dd6f9168e9a6f381a1ea956... ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as https://chromium.googlesource.com/chromium/src/+/fecb2dd6f9168e9a6f381a1ea956...
Message was sent while issue was closed.
On 2017/02/23 17:50:52, Randy Smith (Not in Mondays) wrote: > I don't know anything about components/cookie_config, but I'm happy with the use > of Internals>Network>Cookies if the owners of that directory are. LGTM. Randy, I have some bad news for you ;): blundell2:cookie_config(foo) $ pwd /usr/local/google/home/blundell/chromium/src/components/cookie_config blundell2:cookie_config(foo) $ cat OWNERS blundell@chromium.org rdsmith@chromium.org I was as surprised as you likely are. |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
