There's a few bugs that have to be fixed before you can submit. Otherwise lgtm.
http://codereview.chromium.org/8676/diff/205/8 File src/platform-win32.cc (right): http://codereview.chromium.org/8676/diff/205/8#newcode785 Line 785: _set_abort_behavior(0, _WRITE_ABORT_MSG); How about making this conditional on a command-line flag -- always overriding these seems questionable. http://codereview.chromium.org/8676/diff/205/7 File tools/test.py (right): http://codereview.chromium.org/8676/diff/205/7#newcode175 Line 175: if len(self.crashed) > 0: You never use the contents of the crashed list, only its length. You could use a counter instead. http://codereview.chromium.org/8676/diff/205/7#newcode176 Line 176: print "=== %i tests CRASHED" % len(self.failed) I think you mean len(self.crashed). http://codereview.chromium.org/8676/diff/205/7#newcode404 Line 404: except: Catchall exception handlers in python are extremely evil. You should only catch the kind of exceptions you expect, in this case ImportError. http://codereview.chromium.org/8676/diff/205/7#newcode405 Line 405: Pass It's 'pass', not 'Pass'. Ahem, did you test this? http://codereview.chromium.org/8676/diff/205/7#newcode1075 Line 1075: result.add_option("--win-error-box", help="Display the Windows general protection fault message box", How about only adding this if we're actually on windows, and then removing the win- part of the name. Also, I think error-box is a somewhat confusing name; maybe something like --suppress-dialogs would be better? http://codereview.chromium.org/8676 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
