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

Issue 2824035: Add explicit file rename/cp information to svn rietveld uploads (Closed)

Created:
10 years, 5 months ago by gavinp
Modified:
9 years, 6 months ago
Reviewers:
nsylvain, jam, M-A Ruel
CC:
chromium-reviews, M-A Ruel
Visibility:
Public.

Description

Add explicit file rename/cp information to svn rietveld uploads Right now if you svn cp or svn mv before uploading to rietveld, there's no description in the patch at all of what was done; so you get a diff, but it's for who knows what revision of what prior file to some current version. For code reviews this kinda works (you ask the guy what it was), but for trying to apply patches (ala git patch) this fails badly. This patch tries to add some metadata to the start of a rietveld patch that describes these changes: both to a human (who can read them) and to a potential clever future git-cl (which I'll do next). The metadata looks like this after checking out a test repo, and svn cp -r 1 foo nitz; svn cp bar quux; echo be good>>quux ### BEGIN SVN COPY METADATA #$ svn cp -r 1 foo nitz ### WARNING: note non-trunk copy #$ cp bar quux ### END SVN COPY METADATA Index: nitz Index: quux =================================================================== --- quux (revision 0) +++ quux (working copy) @@ -1,2 +1,3 @@ hi mom hi sister +be good I did a test, and this looks like it works in svn 1.4.4 as well. BUG=none TEST=none Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=51121

Patch Set 1 #

Total comments: 4

Patch Set 2 : '' #

Patch Set 3 : '' #

Unified diffs Side-by-side diffs Delta from patch set Stats (+26 lines, -1 line) Patch
M scm.py View 1 2 2 chunks +26 lines, -1 line 0 comments Download

Messages

Total messages: 7 (0 generated)
gavinp
10 years, 5 months ago (2010-06-28 21:44:29 UTC) #1
jam
lgtm
10 years, 5 months ago (2010-06-28 22:42:13 UTC) #2
nsylvain
LGTM
10 years, 5 months ago (2010-06-28 22:54:30 UTC) #3
M-A Ruel
lgtm with nits http://codereview.chromium.org/2824035/diff/1/2 File scm.py (right): http://codereview.chromium.org/2824035/diff/1/2#newcode824 scm.py:824: assert srcurl[:len(root)] == root assert srcurl.startswith(root) ...
10 years, 5 months ago (2010-06-29 00:49:01 UTC) #4
gavinp
Remediated to review, but in my test upload I found a bug: the warning non-trunk ...
10 years, 5 months ago (2010-06-29 12:39:26 UTC) #5
gavinp
The new upload adds an additional check; the non-trunk warning output is reserved for copies ...
10 years, 5 months ago (2010-06-29 13:10:17 UTC) #6
M-A Ruel
10 years, 5 months ago (2010-06-29 13:16:47 UTC) #7
lgtm

Powered by Google App Engine
This is Rietveld 408576698