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

Issue 115264: Deprecate gcl.GetSVNStatus() for gclient.CaptureSVNStatus() and fix some stat... (Closed)

Created:
11 years, 7 months ago by M-A Ruel
Modified:
9 years, 7 months ago
Reviewers:
Jói Sigurðsson
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Deprecate gcl.GetSVNStatus() for gclient.CaptureSVNStatus() and fix some status information that was lost. This was breaking gclient revert on unversioned files. TEST=ran "for %a in (tests\*test.py) do python %a" and all tests succeeded. Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=15894

Patch Set 1 #

Total comments: 5

Patch Set 2 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+112 lines, -174 lines) Patch
M gcl.py View 3 chunks +4 lines, -68 lines 0 comments Download
M gclient.py View 1 5 chunks +38 lines, -39 lines 0 comments Download
M tests/gcl_unittest.py View 2 chunks +1 line, -65 lines 0 comments Download
M tests/gclient_test.py View 2 chunks +69 lines, -2 lines 0 comments Download

Messages

Total messages: 2 (0 generated)
M-A Ruel
11 years, 7 months ago (2009-05-12 20:50:53 UTC) #1
Jói Sigurðsson
11 years, 7 months ago (2009-05-12 20:59:23 UTC) #2
LGTM with nits

http://codereview.chromium.org/115264/diff/1/5
File gclient.py (right):

http://codereview.chromium.org/115264/diff/1/5#newcode618
Line 618: @file can be a string (one file) or a list of files.
@file -> @files

http://codereview.chromium.org/115264/diff/1/5#newcode620
Line 620: Returns an array of (status, file) tupple."""
tupple -> tuples

http://codereview.chromium.org/115264/diff/1/5#newcode622
Line 622: if files is None:
suggest -> if not files:  (to catch empty list as well)

http://codereview.chromium.org/115264/diff/1/5#newcode629
Line 629: status_letter = {
might be more performant to make this a global or a class member rather than
scoped within the function (?)

http://codereview.chromium.org/115264/diff/1/5#newcode645
Line 645: # TODO(maruel): Find the corresponding strings for X, ~
why this TODO, you seem to have those strings above

Powered by Google App Engine
This is Rietveld 408576698