On 2009/07/14 19:14:35, MarkM wrote:
LGTM modulo comments

http://codereview.appspot.com/93041/diff/2012/3009
File src/com/google/caja/cajita.js (right):

http://codereview.appspot.com/93041/diff/2012/3009#newcode1394
Line 1394: if (obj === null || obj === void 0 ||
"|| obj === void 0" is unnecessary given the rest of the conditional.

Removed.


http://codereview.appspot.com/93041/diff/2012/3008
File tests/com/google/caja/parser/quasiliteral/CajitaRewriterTest.java
(right):

http://codereview.appspot.com/93041/diff/2012/3008#newcode115
Line 115: public void testIn() throws Exception {
Why not promote this to CommonJsRewriterTestCase?

http://codereview.appspot.com/93041/diff/2012/3008#newcode913
Line 913: assertConsistent(
Why not promote this to CommonJsRewriterTestCase?

http://codereview.appspot.com/93041/diff/2012/3007
File
tests/com/google/caja/parser/quasiliteral/DefaultValijaRewriterTest.java
(right):

http://codereview.appspot.com/93041/diff/2012/3007#newcode52
Line 52: public void testStringLength() throws Exception {
Why not promote this to CommonJsRewriterTestCase?

Done.

Also changed valija-cajita.js to avoid TypeErrors around the use of
"in".  Please look those over.

http://codereview.appspot.com/93041

Reply via email to