Skip to content

Ask for web-platform-tests in CONTRIBUTING.md #1767

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 4, 2017
Merged

Ask for web-platform-tests in CONTRIBUTING.md #1767

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba13384
Ask for web-platform-tests in CONTRIBUTING.md
zcorpan Aug 30, 2017
a0953c2
Change policy to must for CR+ and cssom/cssom-view
zcorpan Aug 30, 2017
e86f5eb
Reword per astearns review
zcorpan Aug 30, 2017
4f127d6
Update per review comments
zcorpan Sep 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,19 @@ If you added a contributor by mistake, you can remove them in a comment with:

If you are making a pull request on behalf of someone else but you had no part in designing the
feature, you can remove yourself with the above syntax.

# Tests

For normative changes for any specification in
[CR or later](https://www.w3.org/Style/CSS/current-work) as well as the pre-CR specifications listed
below, a corresponding [web-platform-tests](https://github.com/w3c/web-platform-tests) PR must be
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been asking for a policy like this, so obvious this makes me happy, but I think it's important that there's some leeway here. The "testing is not practical" case will probably come up at least occasionally at CR or later, and if everyone agrees, then this "must" ought not get in the way of progress. I think that if this is downgraded to a "should", then editor's can use their best judgement about when to file issues instead of requiring tests.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I'm reading it, the 'must' is to have a wpt PR or an explanation/issue for followup. It's still at the editor's discretion, but they must do one or the other.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps I'm reading this too much like a format spec, but I'm not able to parse it that way, "a corresponding PR must be provided" sounds quite final. Of course, it's only with RFC 2119 goggles that changing it to "should" helps. Interested to hear how others read this, and don't mind it if other have a more lenient interpretation.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if the intent is the way @astearns is reading it, then it needs to be reworded to be more clear about the either-or.

provided, except if testing is not practical; for other specifications it is usually appreciated.
Typically, both PRs will be merged at the same time. Note that a test change that contradicts the
spec should not be merged before the corresponding spec change. If testing is not practical, please
explain why and if appropriate [file an issue](https://github.com/w3c/web-platform-tests/issues/new)
to follow up later. Add the `type:untestable` or `type:missing-coverage` label as appropriate.

The pre-CR specifications with this testing requirement are currently:

* cssom
* cssom-view
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

css-overflow-3 and css-position-3 will probably fall under this list as well, although I haven't looked at them lately, so fine to leave out for now.