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

Issue 10828291: Fix get_archive to deal with ranges of numbers that appear in the (Closed)

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

Description

Fix get_archive to deal with ranges of numbers that appear in the dartium_archive (see issue # 4484). Committed: https://code.google.com/p/dart/source/detail?r=10614

Patch Set 1 #

Total comments: 6

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+25 lines, -6 lines) Patch
M tools/get_archive.py View 1 3 chunks +25 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Emily Fortuna
8 years, 4 months ago (2012-08-13 22:43:10 UTC) #1
vsm
lgtm with comments https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py File tools/get_archive.py (right): https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py#newcode135 tools/get_archive.py:135: None, we return the latest revision. ...
8 years, 4 months ago (2012-08-13 23:12:57 UTC) #2
Emily Fortuna
8 years, 4 months ago (2012-08-13 23:16:42 UTC) #3
https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py
File tools/get_archive.py (right):

https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py#n...
tools/get_archive.py:135: None, we return the latest revision.
On 2012/08/13 23:12:57, vsm wrote:
> Perhaps clarify the comment.  E.g., "If the revision number is specified but
> unavailable,  find the latest older revision and use that instead."

Done.

https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py#n...
tools/get_archive.py:145: osname, 'num' : revision_num } +
latest[latest.rindex('/'):])
On 2012/08/13 23:12:57, vsm wrote:
> The 'num' doesn't appear the patterns above.  Do you still need it here?

Nope. Fixed!

https://chromiumcodereview.appspot.com/10828291/diff/1/tools/get_archive.py#n...
tools/get_archive.py:163: print latest
On 2012/08/13 23:12:57, vsm wrote:
> Debugging print you left in?

Yes, removed.

Powered by Google App Engine
This is Rietveld 408576698