-
Notifications
You must be signed in to change notification settings - Fork 784
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: Add versioning to plugin add command #916
base: master
Are you sure you want to change the base?
Conversation
12c438e
to
740cdad
Compare
@jthegedus would you have a chance to look over this change soon or would there be another review I could pass this one to? |
@jthegedus sorry for the bump, would you be the best person to review this one or would there be another team member who could look over this change? |
Sorry mate. I'll be reviewing this the next few days. Trying to get on top of the existing PRs so we can move forward on new changes at greater speed. Appreciate the work you have contributed. |
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.
This is looking good so far.
I would like this change to also include what happens when asdf plugin update [<name> | --all]
is called on a ref that is a tag. Currently it will ditch the tag and go back to the HEAD of the default branch.
Is that something you have time to look at @theoretick ?
af16cd2
to
cc8ed51
Compare
@jthegedus sorry for the delay but done with db90b80. I don't love the solution but please let me know what you think! |
Allows user to specify version along with URL when adding plugin to lock to a specific version instead of always relying on HEAD. Relates to asdf-vm#234
db90b80
to
cedbe3e
Compare
403c8c9
to
5f9fdc9
Compare
5f9fdc9
to
0c0f9df
Compare
@jthegedus So I attempted this with 58de320 but it seems this approach is at odds with #800 given asdf/test/plugin_update_command.bats Line 59 in 95f2cdf
|
This feature is super crucial to have minimal security measurements. |
Hi @theoretick , sorry for the late reply here. I agree with @aabouzaid this feature is important for security reasons. However, I'm in the process of rewriting asdf in Golang, and the rewrite should be done in a month or two. I don't see much point in getting this merged right now only to be replaced by the Golang implementation. Would you consider contributing these changes to the Golang implementation in a month or two? |
@Stratus3D sure, if I have capacity then I'd be happy to port it over and excited to see how the rewrite lands. Thanks! |
Just in case anyone is waiting for this, you can start using the community project ASDF Plugin Manager right away and control asdf plugins via |
Summary
Allows user to specify version along with URL when adding plugin to lock
to a specific version instead of always relying on HEAD.
Other Information
Relates to #234