Store files locally in a different location after they've been posted.

Store files locally in a different location after they've been posted.
Committed: https://code.google.com/p/dart/source/detail?r=7424

[vsm, 2012-05-07 20:25:54]

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

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:151: os.mkdir(dir_path)
You can eliminate 'old' from your list and just call makedirs (recursive dir
create) here.  Consider unrolling the loop.  I think a local function is a
cleaner way to factor out redundancy.  E.g.,

make_if_necessary(dir_name)
make_if_necessary(os.path.join('old', dir_name)

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:356: self.result_folder_name]:
I would unroll this loop for clarity also - especially given the if old_dir / if
not old_dir in the body.

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:361: if
self.file_processor.process_file(afile, not old_dir) and \
I'd take process_file out of the conditional.  It's easy to miss side-effecting
code in there.

[Emily Fortuna, 2012-05-08 01:24:51]

PTAL

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

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:151: os.mkdir(dir_path)
On 2012/05/07 20:25:54, vsm wrote:
> You can eliminate 'old' from your list and just call makedirs (recursive dir
> create) here.  Consider unrolling the loop.  I think a local function is a
> cleaner way to factor out redundancy.  E.g.,
> 
> make_if_necessary(dir_name)
> make_if_necessary(os.path.join('old', dir_name)

Done.

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:356: self.result_folder_name]:
On 2012/05/07 20:25:54, vsm wrote:
> I would unroll this loop for clarity also - especially given the if old_dir /
if
> not old_dir in the body.

Done.

https://chromiumcodereview.appspot.com/10383053/diff/2001/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:361: if
self.file_processor.process_file(afile, not old_dir) and \
On 2012/05/07 20:25:54, vsm wrote:
> I'd take process_file out of the conditional.  It's easy to miss
side-effecting
> code in there.

Done.

[vsm, 2012-05-08 03:30:58]

lgtm

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

https://chromiumcodereview.appspot.com/10383053/diff/1002/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:151: print 'Creating output
directory ', dir_path
Did you mean to create old/dir_name here as well if needed?  You deleted that as
well.

[Emily Fortuna, 2012-05-08 16:35:07]

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

https://chromiumcodereview.appspot.com/10383053/diff/1002/tools/testing/perf_...
tools/testing/perf_testing/run_perf_tests.py:151: print 'Creating output
directory ', dir_path
On 2012/05/08 03:30:58, vsm wrote:
> Did you mean to create old/dir_name here as well if needed?  You deleted that
as
> well.

I call this function twice, once for the regular directory  name, and the other
time with old/dir_name (see lines 349-350)

[vsm, 2012-05-08 16:38:09]

Got it.  Still LGTM.