-
Notifications
You must be signed in to change notification settings - Fork 789
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
add starter plugin version feature (Addresses Issue #166) #234
Conversation
lib/commands/plugin-add.sh
Outdated
@@ -28,7 +35,7 @@ plugin_add_command() { | |||
display_error "Plugin named $plugin_name already added" | |||
exit 1 | |||
else | |||
if ! git clone "$source_url" "$plugin_path"; then | |||
if git clone -b "$plugin_branch" "$source_url" "$plugin_path"; then |
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 guess you removed the !
un-intentionally, the script should fail if it's not able to clone the repo.
@skotchpine why branches rather than tags? |
@tomdavidson No reason. Why tags rather than branches? |
Why closed? Vendoring the plugins would be great. Branches just point to HEAD and are a moving target. In many cases tags vendor strong enough that a separate artifact registry is not needed. SCMs typically have built in features for tags https://github.com/asdf-vm/asdf/releases and there is an whole ecosystems of semver'd git tagged releases. |
Any chance we can get this PR re-opened? Or happy to open a fresh one if the original author is unwilling. Having some basic version locking would be great in order to prevent failures due to plugins being updated. In the meantime we have to use something a bit hackier like
the above code should work fine for tags as well given |
If this were to be reopened, I would like to lookup the default branch of the repo instead of assuming I need to finish #800 before we modify any of this too. |
@jthegedus great to see #800 has been merged! I took a look at it and AFAICT in order to get this one ready it sounds like we should be good to omit the else condition here and rely on a clone from the remote's default branch, wdyt? We'd love to get this change into a release, so if you think that's the right direction happy to look into this more. |
@theoretick Yes please! Initial install being either remote HEAD, remote or remote is (I think) enough dials to satisfy people. If you want to open a new PR I will happily review, extra points for tests ;) |
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
Sorry for the delay! Opened #916 as follow-up with a relevant test |
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
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
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
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
Plugin Versioning
This isn't a finished product, but a first draft. It adds a versions to
plugin-add
via git branches. With this change,plguin-add
takes an optional third argument<git-branch>
. If a branch is given and is notmaster
, then the plugin is cloned to<plugin-name>-<git-branch>
instead of<plugin-name>
.