- From: Yuta Kitamura <web-platform-tests-notifications@w3.org>
- Date: Mon, 01 Jul 2013 08:36:04 GMT
- To: public-web-platform-tests-notifications@w3.org
For debugging code: I see some instances in testcommon.js. Also, my comments on "reviewer" field does not seem to be addressed. Some more generic comments: * Avoid using single-letter variable names; try to use a descriptive name wherever it is feasible. * The third argument of `assert_*()` is optional. If it adds some value (such as helping readers to understand the intention of the assertion), it's okay to have, but it's unnecessary otherwise. Messages that just paraphrases the assertion (like `assert_equals(div.parentNode, t.content, 'Wrong foster parent element');`) are not particularly useful. * It looks like you are adding more tests on the way, which looks bad to me. This is the first work for this test suite, and you are in the stage of adjusting the alignment between your views and W3C test standards; I'd be appreciated if you first focus on and start from a very small set of tests that can represent a baseline of the test suite. Also, adding more tests on the way would increase the reviewer's load for no good reason. Generally the tests are getting better; I think I can comment on each test in the next round of review. View on GitHub: https://github.com/w3c/web-platform-tests/issues/135#issuecomment-20269225
Received on Monday, 1 July 2013 08:36:11 UTC