Use os.UserHomeDir() (darwin) and drop dependency on go-homedir (all) #22
Conversation
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
This change applies to code compiled only for darwin, but it also allows to remove a dependency from go.mod which will ease maintenance of downstream projects for any platforms. Use os.UserHomeDir() which is available since go1.12 (already the minimum go version for this project). This allows to drop the dependency on github.com/mitchellh/go-homedir which was needed only for Darwnin. go-homedir still has the following features: * tries very hard to get the location of the home directory when $HOME is not set * avoids use of os/user which dependended on cgo until go1.20 I don't think we really need to go beyond $HOME, but if we really wanted it we could now use os/user.Current() as since go1.20 it uses the libc on Darwin and no longer needs cgo on any platform.
|
Any progress here? Change would be appreciated! |
There was a problem hiding this comment.
PR Overview
This pull request updates the code to use os.UserHomeDir() instead of the external go-homedir dependency, reducing external dependencies and simplifying maintenance for Darwin builds.
- Replaces go-homedir.Dir() with os.UserHomeDir().
- Removes the dependency on github.com/mitchellh/go-homedir from the imports.
Reviewed Changes
| File | Description |
|---|---|
| rootcerts_darwin.go | Updated to use os.UserHomeDir() and removed go-homedir dependency. |
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
rootcerts_darwin.go:43
- Consider using filepath.Join instead of path.Join for constructing file system paths to ensure more robust handling of OS path conventions.
loginKeychain := path.Join(home, "Library", "Keychains", "login.keychain")
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Note: This change applies to code compiled only for darwin, but it also allows to remove a dependency from go.mod which will ease maintenance of downstream projects for any platforms.
Use
os.UserHomeDir()which is available since go1.12 (already the minimum go version for this project). This allows to drop the dependency ongithub.com/mitchellh/go-homedirwhich was needed only for Darwin.go-homedirstill has the following features on Darwin:os/userwhich dependended on cgo until go1.20I don't think we really need to go beyond
$HOME. However if we really wanted it we could now useos/user.Current()as since go1.20 it uses the libc on Darwin and no longer needs cgo on any platform. So we should dropgo-homediranyway.