-
Notifications
You must be signed in to change notification settings - Fork 830
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
feat: Report missing plugins and install plugins in defined order #1970
base: master
Are you sure you want to change the base?
feat: Report missing plugins and install plugins in defined order #1970
Conversation
6c1e051
to
1f7649c
Compare
1f7649c
to
554f6f9
Compare
f9f78d7
to
5272a17
Compare
@@ -100,7 +101,7 @@ func LoadConfig() (Config, error) { | |||
return config, err | |||
} | |||
|
|||
homeDir, err := homedir.Dir() | |||
homeDir, err := os.UserHomeDir() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since you are replacing the library - it is being used here as well
https://github.com/asdf-vm/asdf/pull/1970/files#diff-54c7c1af5fa8d5db4dc49f0e8e80e93ba2b1183ba4d5c9e2e5729e6deae6a3cdL207
There is a related issue
#1923
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed it here since it's a fairly safe change. Nothing else appears to be reading from config.Home
prior to this change and the current implementation of install was already relying on os.UserHomeDir()
. I think removing the other usages merit its own PR. Seems trivial but I'm not confident enough w/ this code base yet to say there aren't edge cases to consider.
@Stratus3D - Ready for some 👀 🙏 |
Summary
This PR adds two features:
Other Information
Sample output: