Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(1613)

Issue 13969043: environs/configstore: change file extension

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 7 months ago by rog
Modified:
10 years, 7 months ago
Reviewers:
mp+187785, mue
Visibility:
Public.

Description

environs/configstore: change file extension I think it's good to have *some* file extension, so we don't have to worry too much about whether we allow environment names to contain periods. The question remains: what extension to choose? Based on Dimiter's suggestion, I've chosen ".jenv", but others' mileages may vary. Let the bikeshedding begin. https://code.launchpad.net/~rogpeppe/juju-core/422-environ-info-extension/+merge/187785 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : environs/configstore: change file extension #

Patch Set 3 : environs/configstore: change file extension #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+5 lines, -3 lines) Patch
A [revision details] View 1 1 chunk +2 lines, -0 lines 0 comments Download
M environs/configstore/disk.go View 1 1 chunk +1 line, -1 line 1 comment Download
M environs/configstore/disk_test.go View 1 2 chunks +2 lines, -2 lines 0 comments Download

Messages

Total messages: 2
rog
Please take a look.
10 years, 7 months ago (2013-09-26 13:48:55 UTC) #1
mue
10 years, 7 months ago (2013-09-26 13:52:51 UTC) #2
Big LGTM!

https://codereview.appspot.com/13969043/diff/5001/environs/configstore/disk.go
File environs/configstore/disk.go (right):

https://codereview.appspot.com/13969043/diff/5001/environs/configstore/disk.g...
environs/configstore/disk.go:54: return filepath.Join(d.dir, "environments",
envName+".jenv")
Good choice. I prefer names/extensions telling what the file is about more than
the format. Always hated all those .xml files.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b