LGTM with 2 comments to fix.
https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc File test/cctest/test-heap-profiler.cc (right): https://chromiumcodereview.appspot.com/9316092/diff/3005/test/cctest/test-heap-profiler.cc#newcode813 test/cctest/test-heap-profiler.cc:813: for (int i = 0; i < 4; i++) On 2012/02/03 14:09:26, Yury Semikhatsky wrote:
On 2012/02/03 13:57:19, Mikhail Naganov (Chromium) wrote: > nit: brackets
Done. Are they required by V8 coding style?
Google C++ coding style says "conditional or loop statements with complex conditions or statements may be more readable with curly braces". In V8, curly braces are in general can be omitted only for trivial early returns from functions. https://chromiumcodereview.appspot.com/9316092/diff/2004/test/cctest/test-heap-profiler.cc File test/cctest/test-heap-profiler.cc (right): https://chromiumcodereview.appspot.com/9316092/diff/2004/test/cctest/test-heap-profiler.cc#newcode831 test/cctest/test-heap-profiler.cc:831: // 1 -> 2, 1 -> 3 Then please add "(note length = 2)" for clarity. https://chromiumcodereview.appspot.com/9316092/diff/2004/test/cctest/test-heap-profiler.cc#newcode836 test/cctest/test-heap-profiler.cc:836: v8::Persistent<v8::Value> objects_[4]; OK, at least, please introduce a constant for the array length. https://chromiumcodereview.appspot.com/9316092/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
