Skip to content

cli/config: use os.UserHomeDir instead of github.com/docker/docker/pkg/homedir#2101

Merged
tiborvass merged 1 commit into
docker:masterfrom
tiborvass:config-less-deps
Sep 24, 2019
Merged

cli/config: use os.UserHomeDir instead of github.com/docker/docker/pkg/homedir#2101
tiborvass merged 1 commit into
docker:masterfrom
tiborvass:config-less-deps

Conversation

@tiborvass

@tiborvass tiborvass commented Sep 23, 2019

Copy link
Copy Markdown
Collaborator

this change was reverted in #2111

Signed-off-by: Tibor Vass [email protected]

This is to avoid too many dependencies as outlined in #2082

Note that, this PR greatly (aka over-) simplifies the homedir logic: it requires HOME envvar to be set, and no longer looks at the current user's homedir via user.Current().HomeDir, as pkg/homedir did.

@tiborvass

Copy link
Copy Markdown
Collaborator Author

Ping @jonjohnsonjr @dmcgowan this is the minimalist implementation.

@vdemeester vdemeester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🤗

@silvin-lubecki silvin-lubecki left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@tiborvass
tiborvass merged commit 0397c2c into docker:master Sep 24, 2019
@GordonTheTurtle GordonTheTurtle added this to the 19.09.0 milestone Sep 24, 2019
@jonjohnsonjr

Copy link
Copy Markdown
Contributor

@tiborvass is it possible to make this not panic in init? Trying to pull this in and some of my unit tests are upset about it.

@tiborvass

Copy link
Copy Markdown
Collaborator Author

@jonjohnsonjr done! #2106

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants