Re: [css-grid] grid-display-grid-001, grid-display-inline-grid-001 and grid-parsing-display-001: review request

Le 2014-11-18 09:40, Manuel Rego Casasnovas a écrit :
> Hi Gérard,
> 
> so here again the 3 tests checking the new values "grid" and
> "inline-grid" for "display" property. I've applied all the changes you
> requested in your first review.
> 
> * grid-display-grid-001:
>   * Source:
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-display-grid-001.xht
>   * View:
> https://rawgit.com/Igalia/csswg-test/css-grid-display-tests/css-grid-1/grid-model/grid-display-grid-001.xht
> 
> * grid-display-inline-grid-001:
>   * Source:
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-display-inline-grid-001.xht
>   * View:
> https://rawgit.com/Igalia/csswg-test/css-grid-display-tests/css-grid-1/grid-model/grid-display-inline-grid-001.xht
> 
> * grid-parsing-display-001:
>   * Source:
> https://github.com/Igalia/csswg-test/blob/css-grid-display-tests/css-grid-1/grid-model/grid-parsing-display-001.xht
> 
> Thanks,
>   Rego

Rego,

The first 2 tests look perfect to me. I wanted to approve them with 
Shepherd but I could not:

http://test.csswg.org/shepherd/search/testcase/spec/css-grid-1/

So, right after <link rel="author" ...> please add

<link rel="reviewer" title="Gérard Talbot" 
href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" /> <!-- 
2014-11-18 -->

In the 3rd test (grid-parsing-display-001.xht) , you may want to make 
the test assertion somewhat better or be a more accurate 
description/reflection of what the test does.

<meta name="assert" content="This test checks that 'grid' and 
'inline-grid' values for 'display' property are parsed correctly. It 
tests that they work as expected from style attribute, CSS and 
JavaScrit." />

The expressions "works correctly" or "works as expected" are very 
frequently read expressions in assert statements but, by themselves, 
they do not describe what is actually being tested. But your assertion 
text is not bad...

----------

Your title mentions "parsing"...

<title>CSS Grid Layout Test: Parsing 'grid' and 'inline-grid' 'display' 
values</title>

but I do not think parsing is the best word here... I could be wrong...

----------

Ok, so, how about:

a) This test verifies 'grid' and 'inline-grid' values for 'display' 
property are supported and that computed value of such 'display' values 
can be accessed via the getComputedStyle DOM method and that such 
'display' values can be retrieved via CSS2Properties interface.

or

b) This test verifies 'grid' and 'inline-grid' values for 'display' 
property are supported and that computed value of such 'display' values 
can be fetched via the getComputedStyle DOM method and that such 
'display' values can be retrieved via CSS2Properties interface.

or something like the above... or maybe

c) This test checks that 'grid' and 'inline-grid' values for 'display' 
property are supported so that DOM methods, specifically 
getComputedValue(), and its associated CSS2Properties interface, can 
fetch such 'display' values.

http://www.w3.org/TR/2000/REC-DOM-Level-2-Style-20001113/css.html#CSS-CSS2Properties-display

There is a difference of meaning between "to be supported" and "to be 
implemented" ...

Rego, you figure this out and reword any of this as you think best (I 
trust you here; your English is probably better than mine ... and you're 
the creator of that test) and then you also add a
<link rel="reviewer" title="Gérard Talbot" 
href="http://www.gtalbot.org/BrowserBugsSection/css21testsuite/" /> <!-- 
2014-11-18 -->
to that test. There. :)

Gérard
-- 
Test Format Guidelines
http://testthewebforward.org/docs/test-format-guidelines.html

Test Style Guidelines
http://testthewebforward.org/docs/test-style-guidelines.html

Test Templates
http://testthewebforward.org/docs/test-templates.html

CSS Naming Guidelines
http://testthewebforward.org/docs/css-naming.html

Test Review Checklist
http://testthewebforward.org/docs/review-checklist.html

CSS Metadata
http://testthewebforward.org/docs/css-metadata.html

Received on Wednesday, 19 November 2014 01:07:04 UTC