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

Use os.UserHomeDir() for Go version 1.12+ #853

Closed
wants to merge 1 commit into from
Closed

Use os.UserHomeDir() for Go version 1.12+ #853

wants to merge 1 commit into from

Conversation

mgrachev
Copy link

@mgrachev mgrachev commented Apr 18, 2019

Hi there!

Use os.UserHomeDir() instead of go-homedir for Go version 1.12+.

@CLAassistant
Copy link

CLAassistant commented Apr 18, 2019

CLA assistant check
All committers have signed the CLA.

@pior
Copy link

pior commented Apr 27, 2019

👍

@jharshman
Copy link
Collaborator

@mgrachev is there an issue with using the https://github.com/mitchellh/go-homedir package?

@mgrachev
Copy link
Author

mgrachev commented May 1, 2019

@jharshman No, this is a dependency and I prefer to minimize dependencies in my projects.

@jharshman
Copy link
Collaborator

@mgrachev Trimming back dependencies is always good, however, the go-homedir package itself is very small and not dependent on Go version. I could see this comment giving the impression to a user that if s/he is using Go 1.12+ that s/he would need to edit the code to use os.UserHomeDir().

@spf13
Copy link
Owner

spf13 commented Jun 7, 2019

@mgrachev I'm confused about this. How does adding this comment to the source help? Are we expecting users to change their dependency source if they are using Go 1.12+?

umarcor added a commit to umarcor/cobra that referenced this pull request Jun 7, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 7, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 7, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 7, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 7, 2019
@mgrachev
Copy link
Author

mgrachev commented Jun 8, 2019

@spf13 Yes, you're right. I've added this comment because I didn't want to drop support for previous versions of Go.

@spf13
Copy link
Owner

spf13 commented Jun 10, 2019

I'm not sure what value there is in adding a comment. I do think there is some value in removing dependencies. One way to do this without breaking older users of Go is to incorporate build tags.
That is, create two alternative files, with a single function in each, one using the dependency and one calling os.UserHomeDir and use build tags to manage which one is included.

@mgrachev mgrachev changed the title Add comment about using os.UserHomeDir() Use os.UserHomeDir() for Go version 1.12+ Jun 11, 2019
@mgrachev
Copy link
Author

@spf13 Done.

umarcor added a commit to umarcor/cobra that referenced this pull request Jul 11, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Aug 23, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 5, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 5, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 5, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 17, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 17, 2019
@jharshman
Copy link
Collaborator

I'm going to close this because it introduces complication without enough benefit.

@jharshman jharshman closed this Oct 7, 2019
@mgrachev mgrachev deleted the feature/add-user-home-dir-comment branch October 8, 2019 08:09
umarcor added a commit to umarcor/cobra that referenced this pull request Oct 9, 2019
umarcor added a commit to umarcor/cobra that referenced this pull request Jul 13, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Aug 29, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Aug 29, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Aug 29, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Sep 25, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Oct 1, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Oct 14, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Oct 18, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Nov 14, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Dec 4, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Dec 29, 2020
umarcor added a commit to umarcor/cobra that referenced this pull request Jan 30, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Jan 30, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 2, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 2, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 2, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 4, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 8, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 9, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 21, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Feb 21, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request May 3, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request May 3, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 29, 2021
umarcor added a commit to umarcor/cobra that referenced this pull request Jun 30, 2021
spf13 pushed a commit that referenced this pull request Jun 30, 2021
@jpeach
Copy link

jpeach commented Jul 5, 2021

Commenting on a closed PR is pretty bad form, but os.UserHomeDir depends on the "$HOME" environment variable being set, whereas go-homedir uses a number of different strategies and basically always works whether "$HOME" is set or not.

@marckhouzam
Copy link
Collaborator

Looks like this has been done by #1337 and released in Cobra 1.2.

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.

7 participants