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

Issue 559003: sync @branchname git support (Closed)

Created:
10 years, 10 months ago by Nasser Grainawi
Modified:
9 years, 7 months ago
CC:
chromium-reviews, chromium-os-reviews_chromium.org, M-A Ruel
Visibility:
Public.

Description

sync @branchname git support Also improve GIT.update error handling and verbosity levels TEST=unit tests BUG=http://crosbug.com/480 BUG=http://crosbug.com/1136 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=39717

Patch Set 1 #

Total comments: 34

Patch Set 2 : response to first round of comments #

Total comments: 16

Patch Set 3 : 2nd round of comments #

Patch Set 4 : Move clone to its own function and add 'files' population and info printing to AttemptRebase #

Total comments: 12

Patch Set 5 : 4th round of comments #

Total comments: 6

Patch Set 6 : Make _AttemptRebase args bools #

Total comments: 6

Patch Set 7 : 6th round of comments #

Total comments: 1

Patch Set 8 : final fixes? #

Patch Set 9 : presubmit tests pass #

Patch Set 10 : add tests #

Patch Set 11 : gclient_scm_tests pass #

Total comments: 2

Patch Set 12 : final set #

Unified diffs Side-by-side diffs Delta from patch set Stats (+317 lines, -44 lines) Patch
M gclient_scm.py View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +283 lines, -38 lines 0 comments Download
M tests/gclient_scm_test.py View 1 2 3 4 5 6 7 8 9 10 11 5 chunks +34 lines, -6 lines 0 comments Download

Messages

Total messages: 33 (0 generated)
Nasser Grainawi
10 years, 10 months ago (2010-02-01 19:06:09 UTC) #1
Nasser Grainawi
This is currently failing the GitWrapperTestCase.testUpdateConflict unit test because we now prompt for a user ...
10 years, 10 months ago (2010-02-01 19:09:58 UTC) #2
Evan Martin
If only you were using merge, it would leave you with nice conflict markers in ...
10 years, 10 months ago (2010-02-01 19:17:09 UTC) #3
Nasser Grainawi
On 2010/02/01 19:17:09, Evan Martin wrote: > If only you were using merge, it would ...
10 years, 10 months ago (2010-02-01 19:22:29 UTC) #4
M-A Ruel
On 2010/02/01 19:09:58, Nasser Grainawi wrote: > This is currently failing the GitWrapperTestCase.testUpdateConflict unit test ...
10 years, 10 months ago (2010-02-01 19:26:15 UTC) #5
Evan Martin
(I would suggest adding a flag like --no-prompts that just leaves your repo in an ...
10 years, 10 months ago (2010-02-01 19:31:06 UTC) #6
Evan Martin
On 2010/02/01 19:31:06, Evan Martin wrote: > (I would suggest adding a flag like --no-prompts ...
10 years, 10 months ago (2010-02-01 19:31:23 UTC) #7
Nasser Grainawi
http://codereview.chromium.org/559003/diff/1/2 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/1/2#newcode189 gclient_scm.py:189: revType = "branch" On 2010/02/01 19:26:15, Marc-Antoine Ruel wrote: ...
10 years, 10 months ago (2010-02-01 21:50:41 UTC) #8
Nasser Grainawi
10 years, 10 months ago (2010-02-01 21:52:17 UTC) #9
M-A Ruel
http://codereview.chromium.org/559003/diff/6001/6002 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/6001/6002#newcode330 gclient_scm.py:330: upstream_branch], align +1 to [ http://codereview.chromium.org/559003/diff/6001/6002#newcode341 gclient_scm.py:341: "rebase? (y)es ...
10 years, 10 months ago (2010-02-02 16:23:38 UTC) #10
Nasser Grainawi
http://codereview.chromium.org/559003/diff/6001/6002 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/6001/6002#newcode330 gclient_scm.py:330: upstream_branch], On 2010/02/02 16:23:39, Marc-Antoine Ruel wrote: > align ...
10 years, 10 months ago (2010-02-02 18:30:42 UTC) #11
Nasser Grainawi
10 years, 10 months ago (2010-02-02 21:42:42 UTC) #12
Nasser Grainawi
Re-org'ed everything but case 3. http://codereview.chromium.org/559003/diff/9001/9002 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/9001/9002#newcode428 gclient_scm.py:428: def _AttemptRebase(self, upstream, files, ...
10 years, 10 months ago (2010-02-02 21:45:21 UTC) #13
M-A Ruel
Still some style nits. I'd like someone else to verify the git usage correctness. http://codereview.chromium.org/559003/diff/9001/9002 ...
10 years, 10 months ago (2010-02-02 21:53:46 UTC) #14
Nasser Grainawi
Any git users available to test this out and/or verify the usage is correct? http://codereview.chromium.org/559003/diff/9001/9002 ...
10 years, 10 months ago (2010-02-03 00:01:56 UTC) #15
Nasser Grainawi
http://codereview.chromium.org/559003/diff/9001/9002 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/9001/9002#newcode251 gclient_scm.py:251: remote_output, remote_err = self.Capture(['remote'] + verbose + ['update'], On ...
10 years, 10 months ago (2010-02-03 19:09:46 UTC) #16
Nasser Grainawi
10 years, 10 months ago (2010-02-03 20:51:38 UTC) #17
M-A Ruel
On 2010/02/03 00:01:56, Nasser Grainawi wrote: > Still not sure I understand why. If we ...
10 years, 10 months ago (2010-02-03 21:00:34 UTC) #18
Nasser Grainawi
10 years, 10 months ago (2010-02-03 21:25:42 UTC) #19
Nasser Grainawi
Ok, took a stab at the args as bools thing. Let me know where it ...
10 years, 10 months ago (2010-02-03 21:27:00 UTC) #20
M-A Ruel
lgtm with 3 nits. Still waiting for someone else to make sure the workflow is ...
10 years, 10 months ago (2010-02-03 21:32:37 UTC) #21
M-A Ruel
I don't know why but chromium-reviews@googlegroups.com keeps on being added.
10 years, 10 months ago (2010-02-03 21:35:21 UTC) #22
Nasser Grainawi
Getting there... http://codereview.chromium.org/559003/diff/15001/7004 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/15001/7004#newcode251 gclient_scm.py:251: # TODO Check that git-svn doesn't try ...
10 years, 10 months ago (2010-02-03 21:47:36 UTC) #23
Nasser Grainawi
10 years, 10 months ago (2010-02-03 21:47:58 UTC) #24
M-A Ruel
Please harass either Evan or Mandeep for the logic. http://codereview.chromium.org/559003/diff/15003/7006 File gclient_scm.py (right): http://codereview.chromium.org/559003/diff/15003/7006#newcode442 gclient_scm.py:442: ...
10 years, 10 months ago (2010-02-03 21:53:08 UTC) #25
Nasser Grainawi
10 years, 10 months ago (2010-02-03 23:05:36 UTC) #26
M-A Ruel
lgtm
10 years, 10 months ago (2010-02-04 16:06:47 UTC) #27
Mandeep Singh Baines
LGTM. Thanks! But could you please add test cases for the four cases.
10 years, 10 months ago (2010-02-04 19:04:41 UTC) #28
Nasser Grainawi
10 years, 10 months ago (2010-02-23 04:59:51 UTC) #29
Nasser Grainawi
10 years, 10 months ago (2010-02-23 05:58:38 UTC) #30
Nasser Grainawi
PTAL
10 years, 10 months ago (2010-02-23 06:04:38 UTC) #31
M-A Ruel
lgtm with a style nit. Congrats! :) http://codereview.chromium.org/559003/diff/20004/21005 File tests/gclient_scm_test.py (right): http://codereview.chromium.org/559003/diff/20004/21005#newcode532 tests/gclient_scm_test.py:532: exception = ...
10 years, 10 months ago (2010-02-23 14:26:15 UTC) #32
Nasser Grainawi
10 years, 10 months ago (2010-02-23 14:36:22 UTC) #33
committing

http://codereview.chromium.org/559003/diff/20004/21005
File tests/gclient_scm_test.py (right):

http://codereview.chromium.org/559003/diff/20004/21005#newcode532
tests/gclient_scm_test.py:532: exception = \
On 2010/02/23 14:26:15, Marc-Antoine Ruel wrote:
> we use this form instead: 
> exception = (
>     "error: Your local changes to 'b' would be overwritten by merge.
Aborting."
>     "\n"
>     "Please, commit your changes or stash them before you can merge.\n")
> 
> (you can also reduce to two lines or split as you want, it's just we don't use
> \)

Doh! I knew that. Oddly though, quite a bit of this file does use \.

Powered by Google App Engine
This is Rietveld 408576698