- From: sgrekhov <web-platform-tests-notifications@w3.org>
- Date: Mon, 01 Jul 2013 07:08:36 GMT
- To: public-web-platform-tests-notifications@w3.org
Please see my notes inline - The commit history is becoming long. By using commands like git rebase --interactive and git add -p you can restructure the commit history. Structure your commit history so it would consist of independent changes that are logically isolated. Don't think it is just a work log (I know, this is the matter of philosophy, but I can't help saying that). If you do this, git push will fail because the tree is diverging, but you can circumvent with git push --force. Not fixed yet. - Write a script test directly within a HTML file. This seems like a convention everyone follows, and it's convenient for readers. Fixed - Write the test metadata by using <meta> and <link> tags. I'm not a big fan of the PROPS() hack you are using, and in my opinion, this makes the tests hard to read. Fixed - Other test suites don't use the section number in directory names, so follow this convention in yours, too. Fixed - test/ directory doesn't make any sense. Fixed - Don't put testharness.js etc. into your directory. Instead, refer them as a site-root relative path (e.g. /resources/testharness.js). Fixed - No need to put test.html that runs all tests. Fixed - Don't use XML's close tag constructs (e.g. <link ... />). You don't have to close elements like link and meta in HTML5. Fixed - Use consistent indentation. I don't intend to force a particular style, but stay consistent within the test suite. testharness.js uses 4-space indent, so you might want to do the same. Fixed but I didn't carefully check all of the source files. There can be some lost files with nonconsistent indention. I'll ckack the files later. This task is in my list - Use consistent naming convention. Don't mix functionName, FUNCTION, FunctionName and function_name. Using different convention is okay if there's a compelling reason, such as "using assert_foobar naming convention to align with testharness.js". We use functionName naming convention as described at http://google-styleguide.googlecode.com/svn/trunk/javascriptguide.xml. But testharness uses assert_foobar convention. - Don't put debugging code. I didn't find any. Could you please point to an example of debugging code? View on GitHub: https://github.com/w3c/web-platform-tests/issues/135#issuecomment-20266223
Received on Monday, 1 July 2013 07:08:43 UTC