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

Issue 9595022: Removes a redundant mode_arch component from paths for generated tests (Closed)

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

Description

Removes a redundant mode_arch component from paths for generated tests Committed: https://code.google.com/p/dart/source/detail?r=4979

Patch Set 1 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -9 lines) Patch
M tools/testing/dart/test_suite.dart View 3 chunks +10 lines, -9 lines 1 comment Download

Messages

Total messages: 2 (0 generated)
zundel
Updates the path processing in createOutputDirectory() Also removes the --out flag on invoking dartc (no ...
8 years, 9 months ago (2012-03-05 16:05:47 UTC) #1
Bill Hesse
8 years, 9 months ago (2012-03-05 16:58:00 UTC) #2
LGTM.

https://chromiumcodereview.appspot.com/9595022/diff/1/tools/testing/dart/test...
File tools/testing/dart/test_suite.dart (right):

https://chromiumcodereview.appspot.com/9595022/diff/1/tools/testing/dart/test...
tools/testing/dart/test_suite.dart:647: debugMode + configuration["arch"];
I think you mean debugMode += configuration["arch"].
Wouldn't 
generatedTestPath = [debugMode +
configuration["arch"]].addAll(generatedTestPath);
be better?

or ( ... == 'debug') ? 
    'Debug_${configuration["arch"] :
    'Release_${configuration["arch"];

Remember they are taking away string concatenation (I think).


Actually - there is no need for this to match the build directory name anymore. 
Just use "${configuration['mode']_configuration['arch']".  You could even add it
to the end of the generatedTestPath, not the beginning.  You just need to make
the path different for different modes and arch.

Powered by Google App Engine
This is Rietveld 408576698