LGTM.

http://codereview.chromium.org/40300/diff/1/2
File src/regexp-delay.js (right):

http://codereview.chromium.org/40300/diff/1/2#newcode304
Line 304:
Was the comment wrong, or has Firefox changed to 1?

http://codereview.chromium.org/40300/diff/1/3
File src/uri.js (right):

http://codereview.chromium.org/40300/diff/1/3#newcode93
Line 93: var value;
How about assigning octets[0], octets[1] and octets[2] to variables,
instead of doing so many array lookups?
Is this auto-generated code?

http://codereview.chromium.org/40300/diff/1/4
File test/mjsunit/regress/regress-244.js (right):

http://codereview.chromium.org/40300/diff/1/4#newcode61
Line 61: assertTrue(e instanceof URIError);
Please add a message to assertTrue calls to identify the erroring test.
In this case, perhaps just the value
  var message = value + ": threw " + e;

http://codereview.chromium.org/40300/diff/1/4#newcode64
Line 64: assertTrue(threw);
Another way to make this assertion, without the need for a variable, is
to insert a 'fail(value);' call at the end of the body of the
try-statement.

http://codereview.chromium.org/40300

--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---

Reply via email to