Conversation
Contributor
There was a problem hiding this comment.
this name does a much better job of communicating what it does. 👍
Member
Author
There was a problem hiding this comment.
Yeah, everything seemed a bit tangled before. Or perhaps: both tangled and fragmented. This time around I've kept larger chunks. I think fewer abstractions will be easier to manage.
We only ever do three things from the outside: 1. Load an existing config, falling back to defaults if it doesn't exist. 2. Update values on a config. 3. Write the config to disc. Everything else is details. In order to be able to set a fake homeDir on this, the New function delegates to a separate load function. This also allows us to avoid potential race conditions in the tests by not setting ENV variables.
kytrinyx
force-pushed
the
refactor-config
branch
from
8696a5f to
7f5be84
Compare
Contributor
|
This looks good to me, it definitely improves readability and its easier to understand whats going on. 👍 |
Member
Author
|
OK, cool. Merging. |
lcowell
pushed a commit
to lcowell/cli
that referenced
this pull request
Jan 25, 2015
Refactor config
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
We only ever do three things from the outside:
Everything else is details.
In order to be able to set a fake homeDir on this, the New function delegates to a separate load function. This also allows us to avoid potential race conditions in the tests by not setting ENV variables.