| Summary: | [css-grid] Move grid-item-alignment tests to WPT folder | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | rmonteriso | ||||||||||||
| Component: | Tools / Tests | Assignee: | Nobody <webkit-unassigned> | ||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||
| Severity: | Normal | CC: | clopez, commit-queue, ews-watchlist, jfernandez, rego, svillar, webkit-bug-importer | ||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||
| Hardware: | Unspecified | ||||||||||||||
| OS: | Unspecified | ||||||||||||||
| Attachments: |
|
||||||||||||||
|
Description
rmonteriso
2020-01-27 10:03:48 PST
Created attachment 388870 [details]
Patch
Comment on attachment 388870 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=388870&action=review > LayoutTests/imported/w3c/ChangeLog:10 > + [css-grid] Move grid-item-alignment tests to WPT folder > + https://bugs.webkit.org/show_bug.cgi?id=206831 > + > + Reviewed by NOBODY (OOPS!). > + > + Add grid-item-alignment tests, checked and adapted, to WPT. > + Add .thirdRowFirstColumn class to grid.css support file and update all tests using this class by removing the duplicated class > + from their <style> section. I guess this changes have been already exported to WPT, and this is a manual import of this tests, right? In that case, telling in the changelog from which WPT commit this changes come can be useful. (In reply to Carlos Alberto Lopez Perez from comment #2) > Comment on attachment 388870 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=388870&action=review > > > LayoutTests/imported/w3c/ChangeLog:10 > > + [css-grid] Move grid-item-alignment tests to WPT folder > > + https://bugs.webkit.org/show_bug.cgi?id=206831 > > + > > + Reviewed by NOBODY (OOPS!). > > + > > + Add grid-item-alignment tests, checked and adapted, to WPT. > > + Add .thirdRowFirstColumn class to grid.css support file and update all tests using this class by removing the duplicated class > > + from their <style> section. > > I guess this changes have been already exported to WPT, and this is a manual > import of this tests, right? > In that case, telling in the changelog from which WPT commit this changes > come can be useful. Thanks, I will add the Chromium CL with which the WPT repo has been updated. Created attachment 389118 [details]
Patch
Created attachment 389120 [details]
Patch
Created attachment 389131 [details]
Patch
Comment on attachment 389131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389131&action=review > LayoutTests/imported/w3c/ChangeLog:11 > + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ? (In reply to Carlos Alberto Lopez Perez from comment #7) > Comment on attachment 389131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389131&action=review > > > LayoutTests/imported/w3c/ChangeLog:11 > > + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 > > nitpick: this should mention the WPT commit (not the Chromium CL). I guess > its https://github.com/web-platform-tests/wpt/commit/b628488204 ? The PR you mention here is related to other css tests, already reimported to WebKit. The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this? Comment on attachment 389131 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=389131&action=review >>> LayoutTests/imported/w3c/ChangeLog:11 >>> + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 >> >> nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ? > > The PR you mention here is related to other css tests, already reimported to WebKit. > The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this? Yeah, it seems I got the wrong commit :) I guess including the link to the WPT PR from to chromium CL would be nice. Is this one https://github.com/web-platform-tests/wpt/pull/21440 ? (In reply to Carlos Alberto Lopez Perez from comment #9) > Comment on attachment 389131 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=389131&action=review > > >>> LayoutTests/imported/w3c/ChangeLog:11 > >>> + Imported to WPT with Chromium CL https://chromium-review.googlesource.com/c/chromium/src/+/2022793 > >> > >> nitpick: this should mention the WPT commit (not the Chromium CL). I guess its https://github.com/web-platform-tests/wpt/commit/b628488204 ? > > > > The PR you mention here is related to other css tests, already reimported to WebKit. > > The tests contained in this patch, instead, have been directly imported with the Chromium CL, without a previous PR to WPT. Is there a better way I could rephrase this? > > Yeah, it seems I got the wrong commit :) > I guess including the link to the WPT PR from to chromium CL would be nice. > Is this one https://github.com/web-platform-tests/wpt/pull/21440 ? A-ha, I see! Yes it is, I'm going to add it with the next patch! Created attachment 389144 [details]
Patch
Comment on attachment 389144 [details] Patch Clearing flags on attachment: 389144 Committed r255419: <https://trac.webkit.org/changeset/255419> All reviewed patches have been landed. Closing bug. |