Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed #68 #73 #83 #86

Closed
wants to merge 10 commits into from
Closed

Fixed #68 #73 #83 #86

wants to merge 10 commits into from

Conversation

didenko
Copy link
Collaborator

@didenko didenko commented Jun 13, 2015

Fix #68 Application exits when marshallConfigReader encounters parsing errors
Fix #73 Current directory used for config if no other specified directories exist
Fix #83 Struct method BindPFlag calls package function SetDefault instead of method of the same name

@didenko
Copy link
Collaborator Author

didenko commented Jun 13, 2015

Hm. A little lost here. According to the Go 1.2 source it indeed does not seem to have sync.Pool. Why is it failing on this build, but not previous ones?

Walked through the interim packages and those did not seem to update references in a way to start this failure to happen.

@dmitshur
Copy link

Hm. A little lost here. According to the Go 1.2 source it indeed does not seem to have sync.Pool. Why is it failing on this build, but not previous ones?

Walked through the interim packages and those did not seem to update references in a way to start this failure to happen.

viper/remote indirectly imports github.com/coreos/go-etcd/etcd, as can be seen on its import graph.

11 days ago, github.com/coreos/go-etcd/etcd was updated to import github.com/ugorji/go/codec which uses sync.Pool.

The previous build was done 14 days ago, that's why it was successful.

@didenko
Copy link
Collaborator Author

didenko commented Jun 14, 2015

Thanks @shurcooL for the breakage explanation!

@didenko didenko mentioned this pull request Jun 14, 2015
@didenko
Copy link
Collaborator Author

didenko commented Jun 15, 2015

This will be fixed after #88 merge.

@kzvezdarov
Copy link
Contributor

Merged #88 - your build should succeed now.
I think this looks good, but I'd like to get an ok to merge from @bep or @spf13 as #73 and #68 change behavior that other projects might be relying on (specifically Hugo might be affected).

@didenko
Copy link
Collaborator Author

didenko commented Jun 22, 2015

Thanks for a bunch of merges today! Pulled this branch forward so that it should be clean to apply to the current spf13/master. Builds/tests cleanly. Makes sense to get @bep and @spf13 feedback.

@didenko
Copy link
Collaborator Author

didenko commented Jul 9, 2015

After talking to @spf13, makes no sense to work around CWD special cases and have the extra tests and complexity. So I will split this PR into ones addressing #68 and #83 issues, close #73 as irrelevant and submit an issue and PR to completely remove CWD from the package. CWD hard-coding is an application, not a library concern.

@didenko didenko closed this Jul 9, 2015
@didenko didenko deleted the fixed_68_73_83 branch August 1, 2015 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants