Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(727)

Issue 10831382: Don't let safari run too far in the past. (Closed)

Created:
8 years, 4 months ago by Emily Fortuna
Modified:
8 years, 4 months ago
Reviewers:
sra1
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Don't let safari run too far in the past. Committed: https://code.google.com/p/dart/source/detail?r=10935

Patch Set 1 : #

Total comments: 4
Unified diffs Side-by-side diffs Delta from patch set Stats (+43 lines, -23 lines) Patch
M tools/testing/perf_testing/run_perf_tests.py View 6 chunks +27 lines, -17 lines 2 comments Download
M tools/testing/webdriver_test_setup.py View 2 chunks +16 lines, -6 lines 2 comments Download

Messages

Total messages: 2 (0 generated)
Emily Fortuna
8 years, 4 months ago (2012-08-18 00:34:08 UTC) #1
sra1
8 years, 4 months ago (2012-08-18 02:59:01 UTC) #2
lgtm

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/perf_...
File tools/testing/perf_testing/run_perf_tests.py (right):

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:353: if platform == 'safari' and
variant == 'dart2js' and \
Preferred Python style is to use parentheses to make expression incomplete at
line end:

if (platform == 'safari' and variant == 'dart2js' and
    int(self.test_runner.current_revision_num) < 10193):

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:785: if
self.test.test_runner.current_revision_num >= 7823:
Since you mention 7823 twice, make it a constant, or at least a local variable
that looks like a constant.

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/webdr...
File tools/testing/webdriver_test_setup.py (right):

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/webdr...
tools/testing/webdriver_test_setup.py:42: "Firefox", action='store_true',
default=False)
This would look better if you put 
  help="xxx"
on its own line, 
  help="xxxx"
       "yyyy"
if necessary.

https://chromiumcodereview.appspot.com/10831382/diff/2001/tools/testing/webdr...
tools/testing/webdriver_test_setup.py:51: help='Perform a buildbot selenium
setup (buildbots have a different' + \
I think the + and \ are both unnecessary

Powered by Google App Engine
This is Rietveld 408576698