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

enhancement: implement --tags semver to use/install a specific version #107

Closed
wants to merge 6 commits into from

Conversation

irkode
Copy link
Contributor

@irkode irkode commented Nov 23, 2024

This PR implements #106

the --tag is

  • added to use and install
  • mutually exclusive with --latest
  • supports semver abbreviations to ease up typing

tests implemented:

  • unit test for the semver detection code
  • vmd/testscripts for parameter calling

additionally:

  • split up cmd_test.go to have separate functions so one can only run a subset

@jmooring
Copy link
Owner

This is still a draft PR so I spent about a minute looking at it. The first thing I noticed was function definitions like these:

func use(useVersionInDotFile bool, useLatest bool, useTag string) error
func install(useLatest bool, useTag string) error

These are a bit confusing, and adding things in the future will make it worse. Please find another way. Maybe we pass a struct, or split those into separate functions, or something else.

@jmooring
Copy link
Owner

jmooring commented Nov 23, 2024

Another option to consider... eliminate the --latest and --tag flags. Maybe.

hvm use
hvm use v1.2.3
hvm use latest

hvm install
hvm install v1.2.3
hvm install latest

Please give it some thought. This removes the mutual exclusivity complication and is easier to document. I have no problem with a breaking change.

@irkode
Copy link
Contributor Author

irkode commented Nov 24, 2024

Thx for having a look at it - The quick check was exactly what I had in mind setting that to draft PR.

The first thing I noticed was function definitions like these:

yes indeed you're right. I noticed that but just continued in the same way done with --latest. Didn't want to refactor that for the PoC

Another option to consider... eliminate the --latest and --tag flags.

in fact I started with passing an argument for the tag but kept the --latest flag. Combining both within a single argument indeed eases up things here and with the abbreviation logic will allow even more lazy typing hvm use .125

I'll target both of your suggestions - take my next Go learning exercise - and come back with a proposal

@irkode irkode closed this Nov 24, 2024
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.

2 participants