Looks pretty good, just a few comments.
http://codereview.chromium.org/7891005/diff/1/src/d8.cc File src/d8.cc (right): http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode930 src/d8.cc:930: ShellThread(int no, char* files) As discussed, let's add comments à la: // Takes ownership of |files|. http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode968 src/d8.cc:968: break; s/break/continue/ http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1189 src/d8.cc:1189: options.parallel_files[parallel_files_set++] = argv[++i]; I'd prefer to have the increment operations on their own lines, but since I can't find that rule in the style guide I leave it up to you. http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1226 src/d8.cc:1226: char* files; We don't like uninitialized pointers. It's safe in this case, but "char* files = NULL;" is better. http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1239 src/d8.cc:1239: } +1 :-) http://codereview.chromium.org/7891005/diff/1/src/d8.cc#newcode1280 src/d8.cc:1280: && options.use_preemption) { Both conditions could go in one line, I think. http://codereview.chromium.org/7891005/ -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
