Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2660083002:
Check in documentation for WPT import and export to //docs. (Closed)
DescriptionCheck in documentation for WPT import/export to //docs.
Copied from:
https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests
and then edited and updated.
BUG=665494
Review-Url: https://codereview.chromium.org/2660083002
Cr-Commit-Position: refs/heads/master@{#446964}
Committed: https://chromium.googlesource.com/chromium/src/+/4f0acca4ba39b6731f7566b8c8af674a22cd78e3
Patch Set 1 #
Total comments: 10
Patch Set 2 : Update documentation in response to feedback #
Total comments: 1
Messages
Total messages: 13 (5 generated)
qyearsley@chromium.org changed reviewers: + foolip@chromium.org
I first updated the copy of the docs at https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests before creating this, so this is a bit different from how it was. Anybody that has time: could you look over this to see if there are any corrections or improvements to be made?
LGTM % nits, now boarding. https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... File docs/testing/web_platform_tests.md (right): https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:3: The W3C provides a large number of conformance tests for different aspects of Nit: s/W3C/Web Platform Tests project/ https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:18: because they are (probably) mostly redundant with Blink's existing tests, and so I don't think this is the reason really. Although it is a concern, are people holding off importing relevant tests because of this? Maybe reword this to explain the concern and what has to be done to overcome it? (A lot of tedious manual work to compare what test are covering.) https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:24: unmodified locally just as easily as we can run the Blink tests Ensure that we Missing punctuation, or was this a bullet list? https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:46: (i.e., you don't need to change the blacklist), all you have to do is run: Suggest blocklist, or the filename itself. https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:91: conflicts? A link to GitHub went missing here, and missing section formatting?
Description was changed from ========== Check in documentation for WPT import and export to //docs. Copied from: https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests BUG=665494 ========== to ========== Check in documentation for WPT import/export to //docs. Copied from: https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests and then edited and updated. BUG=665494 ==========
Thanks for the feedback, now updated https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... File docs/testing/web_platform_tests.md (right): https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:3: The W3C provides a large number of conformance tests for different aspects of On 2017/01/29 at 10:21:47, foolip_UTC7 wrote: > Nit: s/W3C/Web Platform Tests project/ Done https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:18: because they are (probably) mostly redundant with Blink's existing tests, and so On 2017/01/29 at 10:21:47, foolip_UTC7 wrote: > I don't think this is the reason really. Although it is a concern, are people holding off importing relevant tests because of this? > > Maybe reword this to explain the concern and what has to be done to overcome it? (A lot of tedious manual work to compare what test are covering.) This was just copied over from the earlier sites page, so I think that this is out of date. I think that this paragraph is necessary; now removed. https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:24: unmodified locally just as easily as we can run the Blink tests Ensure that we On 2017/01/29 at 10:21:47, foolip_UTC7 wrote: > Missing punctuation, or was this a bullet list? This was a numbered list, but I think it can be a paragraph; now changed. https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:46: (i.e., you don't need to change the blacklist), all you have to do is run: On 2017/01/29 at 10:21:47, foolip_UTC7 wrote: > Suggest blocklist, or the filename itself. Now changed to just have the filename of the script inline with a link to where it is in the repository; also changed the title of this section. https://codereview.chromium.org/2660083002/diff/1/docs/testing/web_platform_t... docs/testing/web_platform_tests.md:91: conflicts? On 2017/01/29 at 10:21:47, foolip_UTC7 wrote: > A link to GitHub went missing here, and missing section formatting? Fixed.
The CQ bit was checked by foolip@chromium.org
lgtm
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": 20001, "attempt_start_ts": 1485763757126460,
"parent_rev": "fbbebbd2d7af02d5f8726ab625895df7f225ab8e", "commit_rev":
"4f0acca4ba39b6731f7566b8c8af674a22cd78e3"}
Message was sent while issue was closed.
Description was changed from ========== Check in documentation for WPT import/export to //docs. Copied from: https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests and then edited and updated. BUG=665494 ========== to ========== Check in documentation for WPT import/export to //docs. Copied from: https://sites.google.com/a/chromium.org/dev/blink/importing-the-w3c-tests and then edited and updated. BUG=665494 Review-Url: https://codereview.chromium.org/2660083002 Cr-Commit-Position: refs/heads/master@{#446964} Committed: https://chromium.googlesource.com/chromium/src/+/4f0acca4ba39b6731f7566b8c8af... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/4f0acca4ba39b6731f7566b8c8af...
Message was sent while issue was closed.
https://codereview.chromium.org/2660083002/diff/20001/docs/testing/web_platfo... File docs/testing/web_platform_tests.md (right): https://codereview.chromium.org/2660083002/diff/20001/docs/testing/web_platfo... docs/testing/web_platform_tests.md:100: [http://testthewebforward.org/docs/]. Some specifics are at I see in https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... that the square brackets show up in the output. Can you remove them?
Message was sent while issue was closed.
On 2017/02/01 at 17:06:12, foolip wrote: > https://codereview.chromium.org/2660083002/diff/20001/docs/testing/web_platfo... > File docs/testing/web_platform_tests.md (right): > > https://codereview.chromium.org/2660083002/diff/20001/docs/testing/web_platfo... > docs/testing/web_platform_tests.md:100: [http://testthewebforward.org/docs/]. Some specifics are at > I see in https://chromium.googlesource.com/chromium/src/+/master/docs/testing/web_plat... that the square brackets show up in the output. Can you remove them? Ah, yes, thanks for pointing that out! Will do that. |
