Uh oh!
There was an error while loading. Please reload this page.
expose config credentials without needing the Cli - #227
Conversation
Codecov Report
@@ Coverage Diff @@## master #227 +/- ##
==========================================
+ Coverage 46.85% 46.98% +0.13%
==========================================
Files 172 172 Lines 11692 11693 +1 ==========================================
+ Hits 5478 5494 +16 + Misses 5902 5882 -20 - Partials 312 317 +5 |
| func LoadDefaultConfigFile(err io.Writer) *configfile.ConfigFile { | ||
| configFile, e := Load(Dir()) | ||
| if e != nil { | ||
| fmt.Fprintf(err, "WARNING: Error loading config file:%v\n", e) |
There was a problem hiding this comment.
I realize this code is only moved, but it looks like a space is missing between : and %v.
dnephin
commented
Jun 22, 2017
Fixed the string format and added a couple more unit tests. |
| } | ||
| // NewConfigFile initializes an empty configuration file for the given filename 'fn' | ||
| func NewConfigFile(fn string) *ConfigFile { |
There was a problem hiding this comment.
nit: maybe in the context of this pkg it should just be called New
vdemeester
left a comment
There was a problem hiding this comment.
LGTM 🦁 with @tonistiigi comment 👼
thaJeztah
commented
Jun 27, 2017
Given that @dnephin is on PTO this week, perhaps someone can carry those nits? |
vdemeester
commented
Jun 27, 2017
I'll carry this one 😉 |
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
379623e to
fda6366CompareSigned-off-by: Vincent Demeester <vincent@sbr.pm>
| expected := types.AuthConfig{ | ||
| Username: "foo", | ||
| Password: "bar", | ||
| Email: "foo@example.com", |
There was a problem hiding this comment.
Do we still have to test for the Email field here?
There was a problem hiding this comment.
Maybe not but could be handled in a follow-up 👼
There was a problem hiding this comment.
If the credential helper stores email, I think we should still test for it.
thaJeztah
left a comment
There was a problem hiding this comment.
had one question, but LGTM overall
ping @tonistiigi PTAL
dnephin
commented
Jun 29, 2017
@vdemeester thanks for making that |
[17.09] bump version to 17.09.0-ce-rc2
Related to moby/moby#30817
Move
GetAllCredentials(), andCredentialsStore()from theDockerClito theConfigFilestruct, so that are easier to use externally.