IMHO: You should also add a description of the differences between version 1 and 2 to run.html.
On Tue, Sep 23, 2008 at 2:09 PM, <[EMAIL PROTECTED]> wrote: > 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 -~----------~----~----~----~------~----~------~--~---
