search

tfutils / tfenv

Terraform version manager
MIT License
4.48k stars 454 forks source link

Add `latest-allowed` option from required_version #309

Closed OJFord closed 1 year ago

OJFord commented 2 years ago

This commit adds an option similar to min-required in that it parses the terraform's required_version, but then uses the most recent version allowed by that spec.

For example, given:

required_version = "~> 0.10.0"

min-required would give us 0.10.0, while latest-allowed will give us 0.10.8 (via latest:^0.10).

The < operator is not implemented (only > >= ~> <=) because it's messy to find the latest version smaller than a given one in list-remote's output, given it may not (yet) exist, and it seems a reasonably assumption/requirement that the version numbers will exist, since, for example:

required_version = "~> 1.0"

expresses the same as:

required_version = "< 2.0.0"

without requiring a version that doesn't (at time of writing) exist.

This assumption/requirement stands for <= too, but again I think that's reasonable since <= 2.0.0 (as opposed to not equal to) seems like a strange version constraint to use before it exists. (And when it does, one can just use 2.0.0.)

Closes #307.

e-power commented 2 years ago

Is it planned to continue this?

OJFord commented 2 years ago

@e-power I am here, and able to make any requested changes, yes.

The maintainers seem to have other priorities at the moment though, no commits for six months & some PRs awaiting review.

OJFord commented 2 years ago

Actually, I just saw there was a release a few days ago. Note the comments there: https://github.com/tfutils/tfenv/releases/tag/v2.2.3

smelchior commented 2 years ago

@OJFord we would also really like this feature! May i ask you to rebase the branch so merging is easier for the maintainers :)?

OJFord commented 2 years ago

@smelchior Done, there weren't actually any conflicts, GitHub's warning was just that it was 'out of date', i.e. there was a commit ('update changelog') on master not on this branch.

I don't think that's what was holding this back though, I wouldn't hold your breath for any merges! Let me know if you run tfenv from this branch / with this patch and run into any issues in the meantime.

@Zordrak Obviously I'm biased by my own PRs, but those aside if you need any help maintaining tfenv, and issue triage etc. I'd be happy to help.

OJFord commented 2 years ago

Hi! Good to hear from you. Sure, I'll look into it.

smelchior commented 2 years ago

I would still really like to see this PR merged, is there something still open I could assist with :)?

OJFord commented 2 years ago

Thanks @smelchior, so would I (and I'm still here and willing to work on this too fwiw 🙂).

I think I added a passing test as was requested, so good to go from my perspective.

If you could (if you're not already) use this branch to test it out manually though I'm sure any feedback (whether you have issues or not) would be appreciated.

smelchior commented 1 year ago

Thanks @OJFord i already did some tests on my side and it works as advertised :) Very nice feature which reduces the hassle of updating the version in 2 places when there is an update!

smelchior commented 1 year ago

Thanks for merging this @Zordrak, that will really help our workflow :) Do you have an estimate when a new release will be made including this feature?

mputilin commented 1 year ago

@Zordrak please release it 🙏

jlestrada commented 1 year ago

@Zordrak any timeline to release this change along with the other commits that were merged in?

Seems like the community (including myself) would be happy to help as well with anything to get this distributed so let us know what we can help with.

timo-reymann commented 1 year ago

Do you have any news when this feature will be released, @Zordrak? :)