LGTM with a few tiny nits (although the stuff was already there before)


https://codereview.chromium.org/448603002/diff/20001/test/base-unittests/cpu-unittest.cc
File test/base-unittests/cpu-unittest.cc (right):

https://codereview.chromium.org/448603002/diff/20001/test/base-unittests/cpu-unittest.cc#newcode34
test/base-unittests/cpu-unittest.cc:34: #if V8_HOST_ARCH_IA32
Using #elif is a bit nicer IMHO, but that's mostly a matter of taste.
I'll leave it up to you.

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/platform/condition-variable-unittest.cc
File test/base-unittests/platform/condition-variable-unittest.cc
(right):

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/platform/condition-variable-unittest.cc#newcode273
test/base-unittests/platform/condition-variable-unittest.cc:273:
Thread** threads = new Thread*[thread_count];
Use ScopedVector? Or do we explicitly want to avoid our utilities here?
I'm fine either way, but we should reach some consensus.

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/platform/platform-unittest.cc
File test/base-unittests/platform/platform-unittest.cc (right):

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/platform/platform-unittest.cc#newcode24
test/base-unittests/platform/platform-unittest.cc:24: #if V8_OS_WIN
Again: #elif (perhaps)

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/utils/random-number-generator-unittest.cc
File test/base-unittests/utils/random-number-generator-unittest.cc
(right):

https://codereview.chromium.org/448603002/diff/40001/test/base-unittests/utils/random-number-generator-unittest.cc#newcode33
test/base-unittests/utils/random-number-generator-unittest.cc:33:
EXPECT_TRUE(b == false || b == true);
:-)

https://codereview.chromium.org/448603002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to