change default extension of configuration file to json

[lcowell]

• renames configuration file when calling ToFile and FromFile
• does nothing if user passes a -c option to the cli
• does nothing if it would clobber a user's files
• notifies user once change is made

I'm new to go and contributing because I want practice and want to learn. Feedback is welcome.

This should resolve issue #39

[lcowell]

The Travis build failure appears to be an issue with go vet not being available.

$ go vet ./...
go tool: no such tool "vet"; to install:

[kytrinyx]

D'oh. I added that this morning, and then we had a critical issue at work and I haven't had a chance to check it. Let me remove that for now.

I need to look at this tomorrow morning—so fried right now.

Thanks for working on this!

[kytrinyx]

Per the golang style guide, all exported functions should be commented (see the style of the other exported comments in this package). For now I've only been trying to clean up the config package–main has larger issues than style at the moment.

http://golang.org/doc/effective_go.html#commentary

[kytrinyx]

This looks good overall. I'm slowly trying to refactor my way to a cleaner design in this thing, so all the style comments are mostly just a conversation starter about where I want to go eventually.

I've been running golint on the config package:

$ golint config/*go

It might sometimes complain about things that aren't really problems, but for the most part it really helps clean up some basic go idiom things.

[kytrinyx]

If this is used outside the package, then I would name it Normalize() (because then it would be called with config.NormalizeConfig() and there's a stutter). Here, since it's only used inside the package I wouldn't export it, and I would name it normalizeFilename.

[lcowell]

Thanks for taking the time to put this feedback together. I've implemented most of your suggestions. BTW should I be rebasing or merging cli/master in to this feature branch ?

[kytrinyx]

Yeah, go ahead and rebase this onto master.

This looks good. I can change the bit around messaging the user after it's merged in to master.

[lcowell]

Sorry, silly question, but do you mean for me to call git rebase master from my json-extension branch once I've update my master to be in sync with yours ?

[kytrinyx]

Yeah, there are a few ways you could do this:

1. Rebase by pulling with rebase from the upstream master:

# assuming you are on json-extension
git pull --rebase upstream master

1. Fetch the remote and rebase onto the tag for the upstream master:

# assuming you are on json-extension
git fetch upstream && git rebase upstream/master

1. Update your master then rebase onto it:

git checkout master
git pull upstream master # assuming you have not changed your master
git checkout json-extension
git rebase master

[lcowell]

Thanks for the explanation, I definitely learned something from that. After performing this I'm getting branches diverged status. I can easily undo this operation (thanks reflog!).

/G/s/g/e/cli (json-extension) $ git pull --rebase upstream master
<snip>
~/G/s/g/e/cli (json-extension) $ git status
On branch json-extension
Your branch and 'origin/json-extension' have diverged,
and have 13 and 11 different commits each, respectively.
  (use "git pull" to merge the remote branch into yours)

I can pull, but then I get a merge commit.
json-extension will already merge cleanly in to master already, so does it makes sense to just merge it in ? What would you do here ? Thanks!

[kytrinyx]

Branches diverged is normal in this scenario (sorry, I should have mentioned that), and the fix is to push with --force:

git push --force origin json-extension

That will put whatever is on your local branch up on the remote branch, and it automatically updates the pull request.

[kytrinyx]

I should probably mention that pushing with --force is a terrible idea on master, and on any branch where you're collaborating with someone. For pull requests it's a common scenario, though, so you'll end up seeing it more often in github-specific open source projects.

[lcowell]

Excellent! Thanks for your help! I did a push --force, so unless you see something else, I don't think there's any more to do on this pull request.

[kytrinyx]

This looks great, thank you!