- From: Florian Rivoal via GitHub <sysbot+gh@w3.org>
- Date: Wed, 11 Mar 2020 01:02:09 +0000
- To: public-css-archive@w3.org
Caveat about this review: This is a large PR, and I went through it relatively quickly. It is quite likely that I missed some things. With that said, overall it looks good, and here are my detailed commit-per-commit comments: * [First commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/8c9c7ab16d6bb8787dbcf6d933c5d0df6667b7a1) seems ok, except that some `propinst-*` classes got dropped in the build output (e.g. `propinst-pause-before` in aural.html). I am not sure that they served a purpose, so maybe it's OK that they're gone, but if they did something useful, it would be good to figure out what killed them and to bring them back. * [Second commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/bef9f1cd3938b9a240dfa8010f5fe919a2bd13f2) OK * [Third commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/0eaa16d4865139926782e47adc4e1f19b83324c0) OK (should have cought it in first commit review, but didn't. Sorry) * [Fourth commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/2ce6e0ff2854961a0996b49551957cd1c9d7f387) OK * [5th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/8f819d91127cc5a58c41110015aa3cc530e97425) OK * [6th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/d12c8ecd1dec7c8c99e848bce3642ac0b7ff34e2) OK, except for one small mistake in a comment, see https://github.com/w3c/csswg-drafts/pull/4850#pullrequestreview-372409072 * [7th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/e13136fd4891eaaddabaf39b0775f26470a5d02d) somewhat subjective, but OK * [8th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/e01302f038a3a7b70adf86d7d6ad022562834bc5) The statement that no UA would do anything with bare links is not correct. For instance, presto-based opera would show the `rel=first` and `rel=index` links in the info panel. While that UA is no longer relevant, there may be others. That said, I am not sure this will cause issues for anyone in practice. But do we solve anything by removing them? * [9th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/519bc2b9a8b6cda75149a33f471b56956bea0f39) OK * [10th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/2e47108a037394e8d238d15f47b4095d43bbb1fd) OK * [11th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/38183e52e61244dc1e2db3b365885215fa6d54c2) OK * [12th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/cb074cdb00041f0059a365ff12ef23d2ad947f43) It seems fine, but I am not sure what the benefit of switching from `<pre><code>…</code></pre>` to `<pre>…</pre>` is. As far as I can tell, bikeshed happily accepts both. Not a blocker, but I'm curious. * [13th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/21aee42aead5d256033ec088ce86702324e5a482) OK * [14th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/11c4ba4adc6ef1aec80554badef6d2102198de3c) OK * [15th commit](https://github.com/w3c/csswg-drafts/pull/4850/commits/c608c11ec64d70a81ed5ec8533c468cc69ae57d8) Not a blocker, but seems unfortunate, this was useful. I wonder if it can be preserved in a different form. -- GitHub Notification of comment by frivoal Please view or discuss this issue at https://github.com/w3c/csswg-drafts/pull/4850#issuecomment-597391881 using your GitHub account
Received on Wednesday, 11 March 2020 01:02:11 UTC