LGTM with a few important comments:
http://codereview.chromium.org/4401/diff/1/3 File benchmarks/base.js (right): http://codereview.chromium.org/4401/diff/1/3#newcode149 Line 149: this.runner.NotifyResult(this.name + BenchmarkSuite.version, I understand why you want to do this, but it looks ugly. Let's put a description on the changes from version 1 -> 2 on the page instead. http://codereview.chromium.org/4401/diff/1/8 File benchmarks/crypto.js (right): http://codereview.chromium.org/4401/diff/1/8#newcode1680 Line 1680: TEXT = TEXT + TEXT; Why is this necessary? Why not just extend the original text if you find that it's too short? http://codereview.chromium.org/4401/diff/1/5 File benchmarks/run.html (right): http://codereview.chromium.org/4401/diff/1/5#newcode86 Line 86: status.innerHTML = "Score (Version " + BenchmarkSuite.version + "): " + score; Revert this change. It doesn't look good. Let's describe the changes on the page instead. http://codereview.chromium.org/4401/diff/1/2 File benchmarks/run.js (right): http://codereview.chromium.org/4401/diff/1/2#newcode44 Line 44: print('Score (V8 Benchmark Suite Version ' + BenchmarkSuite.version + '): ' + score); Change the output here to 'Score (version y): xxx'. It's shorter and it looks better. http://codereview.chromium.org/4401 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
