LGTM

Could you update the change with these comments and I will land the change for
you. Did the use of the PreCompilation API land? If it did, we need to
coordinate landing this change so we don't break Chromium when pushing a new
version of V8.


http://codereview.chromium.org/2118010/diff/1/4
File src/api.cc (right):

http://codereview.chromium.org/2118010/diff/1/4#newcode1110
src/api.cc:1110: static const int kCharToUnsignedFactor =
sizeof(unsigned) / sizeof(char);
Just use sizeof(unsigned)?

http://codereview.chromium.org/2118010/diff/1/4#newcode1113
src/api.cc:1113: // Note that Script::New() will ASSERT if this empty
ScriptData is used.
This note does not seem necessary. Many API entries rely on the user
passing in non-null handles.

http://codereview.chromium.org/2118010/diff/1/4#newcode1120
src/api.cc:1120: // will fail causing Script::New() to ASSERT.
Is the note needed? Your API comment already says that the ScriptData is
architecture dependent.

http://codereview.chromium.org/2118010/diff/1/2
File test/cctest/test-api.cc (right):

http://codereview.chromium.org/2118010/diff/1/2#newcode8039
test/cctest/test-api.cc:8039: const char *script = "function foo(a) {
return a+1; }";
Placement of the '*' here and in the rest of the test code.

http://codereview.chromium.org/2118010/show

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to