[WPT Sync] Ignore OWNERS files during both import and export. |
|||||||
Issue descriptionBackground: in Chromium, we want to keep track of Chromium owners, teams and components for layout test directories, including wpt. However, wpt has its own set of OWNERS files, where owners are specified as GitHub usernames, and Chromium teams/components are not relevant. From email thread: sergiyb@: > IMHO a simpler solution would be to not sync OWNERS files at all and let files in Chromium only contain team/component tags while files in GitHub contain @username entries. To avoid confusion we can add a presubmit check forbidding adding owners to Chromium repo unless it's a team/component tag. This would mean treating OWNERS files similar to the way that baselines (-expected.txt files) are treated now -- not exportable, and not deleted upon import. Currently, the importer already doesn't import OWNERS files[1]. So the changes that will need to be made are: - Import: do not delete OWNERS files on import[2]. - Export: do not consider OWNERS files as exportable[3]. [1] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_copier.py#148 [2] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py#239 [3] https://chromium.googlesource.com/chromium/src/+/22169060c5f292589c918345fd820dd8c8a4ac49/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit.py#86
,
Mar 17 2017
,
Mar 17 2017
P1 since it's blocking another P1.
,
Mar 20 2017
Not captured in the description is translating a GitHub @username to someone@chromium.org. For example, I wouldn't mind seeing reviews in external/wpt/fullscreen, where I am in the upstream OWNERS file, and I'm not sure if it's possible to subscribe to components in reviews?
,
Mar 20 2017
Since we currently don't sync OWNERS anyway, I think adding a sync for individual OWNERS is a separate problem. It is also non-trivial since one would need to keep a mapping somewhere and make sure that it stays up-to-date with actual owners. Therefore, I think this should not be implemented as part of this bug. If in the future we will decide to add support for individual OWNERS, we can then replace the presubmit check mentioned in the description with the one that will check the mapping before adding owners.
,
Mar 20 2017
Keeping the OWNERS files completely separate (no mapping between them) for now SGTM. Note that the upstream files have an entirely different semantic from our notion of "OWNERS" - they're really a WATCHLIST file: https://github.com/w3c/web-platform-tests/blob/master/README.md#test-review.
,
Mar 28 2017
,
Mar 28 2017
So what's the resolution here? We have OWNERS files in the chromium repo that only contain teams/bug components and that we don't sync upstream? If so, that SGTM.
,
Mar 28 2017
Yes, that's the resolution and the above CL will implement this. OWNERS files are NOT synchronized both ways.
,
Mar 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/54b5563663f325d159f01efa846e74c803c65119 commit 54b5563663f325d159f01efa846e74c803c65119 Author: sergiyb <sergiyb@chromium.org> Date: Tue Mar 28 19:38:45 2017 Ignore OWNERS files during external/wpt import and export R=qyearsley@chromium.org BUG= 702283 Review-Url: https://codereview.chromium.org/2779053002 Cr-Commit-Position: refs/heads/master@{#460190} [modify] https://crrev.com/54b5563663f325d159f01efa846e74c803c65119/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit.py [modify] https://crrev.com/54b5563663f325d159f01efa846e74c803c65119/third_party/WebKit/Tools/Scripts/webkitpy/w3c/chromium_commit_unittest.py [modify] https://crrev.com/54b5563663f325d159f01efa846e74c803c65119/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer.py [modify] https://crrev.com/54b5563663f325d159f01efa846e74c803c65119/third_party/WebKit/Tools/Scripts/webkitpy/w3c/test_importer_unittest.py
,
Mar 28 2017
CL has landed, but where is this service running? Do I need to deploy changes to it manually or will it update itself?
,
Mar 28 2017
,
Mar 28 2017
The service consists of a couple builders running recipes on the chromium.infra.cron master: Import: https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller Export: https://build.chromium.org/p/chromium.infra.cron/builders/wpt-exporter Currently, the import only happens once per day, and today is a special day since the csswg-test repo just got merged into web-platform-tests; export script is run continuously. No need to deploy changes manually; since the recipes run scripts in chromium/src, and so the next time it starts the changes should be in effect :-) This means we should be able to start adding OWNERS files into subdirectories of LayoutTests/external/wpt/, and neither the importer or exporter should touch the new files.
,
Mar 30 2017
Thanks for explaining. I've landed hundreds of OWNERS files here: http://crrev.com/2751113002 and looks like they are not exported, e.g. compare https://chromium.googlesource.com/chromium/src/+/master/third_party/WebKit/LayoutTests/external/wpt/FileAPI/OWNERS and https://github.com/w3c/web-platform-tests/blob/master/FileAPI/OWNERS.
,
Jul 3 2017
,
Jul 3 2017
|
|||||||
►
Sign in to add a comment |
|||||||
Comment 1 by serg...@chromium.org
, Mar 17 2017Owner: serg...@chromium.org
Status: Assigned (was: Available)