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

Issue 675713002: Make git auto-svn idempotent. (Closed)

Created:
6 years, 2 months ago by agable
Modified:
6 years, 2 months ago
Reviewers:
pgervais
CC:
chromium-reviews, Dirk Pranke, cmp-cc_chromium.org, iannucci+depot_tools_chromium.org
Project:
tools
Visibility:
Public.

Description

Make git auto-svn idempotent. By directly using git-config rather than git-svn-init, this ensures that crazy values don't get set. It could be "safer" (e.g. check to see if any other git-svn configuration already exists and prompt before overwriting it), but I think that simplicity is better here. R=pgervais@chromium.org BUG=425838 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=292640

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1 line, -1 line) Patch
M git_auto_svn.py View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
agable
6 years, 2 months ago (2014-10-23 10:39:46 UTC) #1
pgervais
On 2014/10/23 10:39:46, agable wrote: lgtm, but it has the downside of using 'internals' of ...
6 years, 2 months ago (2014-10-23 15:38:21 UTC) #2
agable
On 2014/10/23 15:38:21, pgervais wrote: > On 2014/10/23 10:39:46, agable wrote: > > lgtm, but ...
6 years, 2 months ago (2014-10-24 08:12:15 UTC) #3
agable
Committed patchset #1 (id:1) manually as 292640.
6 years, 2 months ago (2014-10-24 08:13:07 UTC) #4
Sam Clegg
On 2014/10/24 08:13:07, agable wrote: > Committed patchset #1 (id:1) manually as 292640. I've getting ...
6 years, 2 months ago (2014-10-24 23:47:12 UTC) #5
pgervais
6 years, 2 months ago (2014-10-24 23:58:17 UTC) #6
Message was sent while issue was closed.
On 2014/10/24 23:47:12, Sam Clegg wrote:
> On 2014/10/24 08:13:07, agable wrote:
> > Committed patchset #1 (id:1) manually as 292640.
> 
> I've getting presubmit errors due to prefix now not be referenced:
> https://codereview.chromium.org/667793005/
> 
> Should I just remove this variable in my CL?

Could you file another CL just for that? I'll lgtm it.

Powered by Google App Engine
This is Rietveld 408576698