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

Issue 9845029: Fixes two type warnings in test_runner.dart (Closed)

Created:
8 years, 9 months ago by zundel
Modified:
8 years, 9 months ago
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fixes two type warnings in test_runner.dart Committed: https://code.google.com/p/dart/source/detail?r=5798

Patch Set 1 #

Total comments: 2

Patch Set 2 : added comment to explain fields[7] #

Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -2 lines) Patch
M tools/testing/dart/test_runner.dart View 1 1 chunk +4 lines, -2 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
zundel
8 years, 9 months ago (2012-03-23 18:59:42 UTC) #1
Bill Hesse
lgtm https://chromiumcodereview.appspot.com/9845029/diff/1/tools/testing/dart/test_runner.dart File tools/testing/dart/test_runner.dart (right): https://chromiumcodereview.appspot.com/9845029/diff/1/tools/testing/dart/test_runner.dart#newcode334 tools/testing/dart/test_runner.dart:334: errors.add(fields[7]); Perhaps add a comment that the relevant ...
8 years, 9 months ago (2012-03-23 22:16:07 UTC) #2
zundel
8 years, 9 months ago (2012-03-24 01:06:39 UTC) #3
https://chromiumcodereview.appspot.com/9845029/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_runner.dart (right):

https://chromiumcodereview.appspot.com/9845029/diff/1/tools/testing/dart/test...
tools/testing/dart/test_runner.dart:334: errors.add(fields[7]);
On 2012/03/23 22:16:08, Bill Hesse wrote:
> Perhaps add a comment that the relevant data is in field 8?  Or add constants
> for the field positions?
> 
> Why did this not cause failures?  Do we only ever use errors.length and
> staticWarnings.length?

Added constants.

In the case of multitests, it looks like only the lengths are checked.  Still, I
want to put something meaningful in there.

Powered by Google App Engine
This is Rietveld 408576698