http://codereview.chromium.org/149530/diff/1/2 File benchmarks/v1/run.html (right):
http://codereview.chromium.org/149530/diff/1/2#newcode67 Line 67: background:#ffffd9; On 2009/07/14 08:01:42, Kasper Lund wrote: > Space after :. Done. http://codereview.chromium.org/149530/diff/1/2#newcode106 Line 106: function showWarning() { On 2009/07/14 08:01:42, Kasper Lund wrote: > showWarning -> ShowWarning. It's still a weird name, because it only shows the > warning if it's obsolete. Maybe change the name to reflect this? > ShowWarningIfObsolete? Done. http://codereview.chromium.org/149530/diff/1/2#newcode108 Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) + "/run.html"; On 2009/07/14 08:01:42, Kasper Lund wrote: > Spaces around +. Done. http://codereview.chromium.org/149530/diff/1/2#newcode108 Line 108: var latesturl = "../v" + (parseInt(BenchmarkSuite.version)+1) + "/run.html"; On 2009/07/14 08:01:42, Kasper Lund wrote: > Weird name. Why not latest_url? Done. http://codereview.chromium.org/149530/diff/1/2#newcode111 Line 111: } else { On 2009/07/14 08:01:42, Kasper Lund wrote: > else if (...) instead of else { if (...) Done. http://codereview.chromium.org/149530/diff/1/2#newcode117 Line 117: xmlhttp.open('GET', latesturl, true); On 2009/07/14 08:01:42, Kasper Lund wrote: > It's a bit weird that you even try this if xmlhttp is null. Well, I'll catch the null exception anyhow. But I can add an explicit null check for robustness against future changes. http://codereview.chromium.org/149530/diff/1/2#newcode119 Line 119: if (xmlhttp.readyState==4 && xmlhttp.status==200) { On 2009/07/14 08:01:42, Kasper Lund wrote: > Spaces around ==. Done. http://codereview.chromium.org/149530/diff/1/2#newcode141 Line 141: Warning! This is not the latest version of V8 Benchmark Suite. For the latest version please visit <a href="http://v8.googlecode.com/svn/data/benchmarks/current/run.html";>this page</a>. On 2009/07/14 08:01:42, Kasper Lund wrote: > Maybe wrap the line a bit differently? 80 characters per line. How about > changing the wording to something like: Warning! This is not the latest version > of the V8 benchmark suite. Consider running the <a>latest version</a>. Done. http://codereview.chromium.org/149530 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
