12 years, 9 months ago
(2011-06-30 20:37:54 UTC)
#6
thanks, committing.
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
File syzygy/scripts/benchmark/zip_benchmark.py (right):
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
syzygy/scripts/benchmark/zip_benchmark.py:93: eggs = '\n'.join([' \'%s\',' %
os.path.basename(egg) for egg in eggs])
On 2011/06/30 20:23:34, Roger McFarlane wrote:
> drop the escaped quotes and use %r instead of %s. This will insert the python
> representation of the string (i.e., quoted and properly escaped).
Neat, I didn't know about that. Thanks!
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
syzygy/scripts/benchmark/zip_benchmark.py:97: open(path, 'wb').write(output)
On 2011/06/30 20:23:34, Roger McFarlane wrote:
> make sure the file gets flushed/closed since you need to capture it in the zip
> later.
>
> with open(...) as f:
> f.write(output)
Done.
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
syzygy/scripts/benchmark/zip_benchmark.py:112: temp_file = StringIO.StringIO()
On 2011/06/30 20:23:34, Roger McFarlane wrote:
> cStringIO.StringIO() is generally a better (more performant) choice and is a
> drop-in replacement for StringIO.StringIO if you don't need to derive from it.
Done.
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
syzygy/scripts/benchmark/zip_benchmark.py:114: zip = zipfile.ZipFile(temp_file,
'w', zipfile.ZIP_DEFLATED)
On 2011/06/30 20:23:34, Roger McFarlane wrote:
> zip = ...
> try:
> for ...
> finally:
> zip.close()
>
> -- or --
>
> with contextlib.closing(zipfile.ZipFile(...)) as zip:
> for ...
Done.
http://codereview.appspot.com/4661063/diff/1/syzygy/scripts/benchmark/zip_ben...
syzygy/scripts/benchmark/zip_benchmark.py:126: open(output_file,
'wb').write(output)
On 2011/06/30 20:23:34, Roger McFarlane wrote:
> Similarly:
>
> with open(output_file, 'wb') as stream:
> stream.write(output)
>
> Maybe a small helper since you do this in a couple of places.
>
> def _WriteFile(path, contents):
> with open(path, 'wb') as f:
> f.write(contents)
Done.
Issue 4661063: Fix build configuration for benchmark zip file.
(Closed)
Created 12 years, 9 months ago by Siggi
Modified 12 years, 9 months ago
Reviewers: chrisha, Roger McFarlane
Base URL: http://sawbuck.googlecode.com/svn/trunk/
Comments: 10