Issue 391052: Group SCM-specific functions in classes to simplify generalization of the interface.

Issue 391052: Group SCM-specific functions in classes to simplify generalization of the interface. (Closed)

Created:
11 years, 1 month ago by M-A Ruel

Modified:
9 years, 7 months ago

Reviewers:
Dirk Pranke

CC:
chromium-reviews_googlegroups.com, Mandeep Singh Baines

Visibility:
Public.

Description

Group SCM-specific functions in classes to simplify generalization of the interface. TEST=none BUG=none Move scm functions into a class to make it simpler to manage.

Patch Set 3 : Fix and cleanups #

Patch Set 4 : One more fix #

Total comments: 7

Created: 11 years, 1 month ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+698 lines, -680 lines)			Patch
M	gcl.py	View	1 2	12 chunks	+13 lines, -44 lines	1 comment	Download
M	gclient.py	View	1 2	3 chunks	+4 lines, -4 lines	2 comments	Download
M	gclient_scm.py	View	1 2	19 chunks	+47 lines, -62 lines	0 comments	Download
M	gclient_utils.py	View	1	7 chunks	+11 lines, -11 lines	0 comments	Download
M	git_cl_hooks.py	View	1 2 3	2 chunks	+3 lines, -4 lines	0 comments	Download
M	presubmit_support.py	View		13 chunks	+17 lines, -17 lines	0 comments	Download
M	revert.py	View		1 chunk	+1 line, -1 line	0 comments	Download
M	scm.py	View	1 2 3	1 chunk	+401 lines, -322 lines	4 comments	Download
M	tests/gcl_unittest.py	View		4 chunks	+20 lines, -27 lines	0 comments	Download
M	tests/gclient_scm_test.py	View	1	11 chunks	+43 lines, -33 lines	0 comments	Download
M	tests/gclient_test.py	View		2 chunks	+5 lines, -12 lines	0 comments	Download
M	tests/presubmit_unittest.py	View		21 chunks	+65 lines, -59 lines	0 comments	Download
M	tests/revert_unittest.py	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	tests/scm_unittest.py	View		11 chunks	+57 lines, -33 lines	0 comments	Download
M	tests/super_mox.py	View		1 chunk	+1 line, -1 line	0 comments	Download
M	tests/trychange_unittest.py	View		1 chunk	+3 lines, -3 lines	0 comments	Download
M	trychange.py	View	1 2 3	3 chunks	+5 lines, -46 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

M-A Ruel

This change is quite tricky. I can probably split it in 3 if you prefer. ...

11 years, 1 month ago (2009-11-13 21:41:07 UTC) #1

Dirk Pranke

LGTM, apart from the general style questions ... http://codereview.chromium.org/391052/diff/2020/2022 File gclient.py (right): http://codereview.chromium.org/391052/diff/2020/2022#newcode752 Line 752: ...

11 years, 1 month ago (2009-11-13 22:42:53 UTC) #2

M-A Ruel

11 years, 1 month ago (2009-11-14 01:15:00 UTC) #3

msb, FYI.

http://codereview.chromium.org/391052/diff/2020/2022
File gclient.py (right):

http://codereview.chromium.org/391052/diff/2020/2022#newcode752
Line 752: modified_files = gclient_scm.scm.SVN.CaptureStatus(e_dir)
gclient should never reference a SCM directly. It should use
gclient_scm.CreateSCM. Probably an oversight when msb@ changed that area.

http://codereview.chromium.org/391052/diff/2020/2028
File scm.py (right):

http://codereview.chromium.org/391052/diff/2020/2028#newcode1
Line 1: # Copyright (c) 2006-2009 The Chromium Authors. All rights reserved.
Yes that's probably what I'm going to do but not now. :)

Expand Messages | Collapse Messages