add support for mise.toml version file type#374
Conversation
paulo-ferraz-oliveira
left a comment
paulo-ferraz-oliveira
left a comment
There was a problem hiding this comment.
Left 3 minor comments. Once those are solved and if CI's passing this should be good for merging. Sorry it took me so long to get back to this.
mruoss
force-pushed
the
mise-toml
branch
3 times, most recently
from
e37f85b to
ffcf1e2
Compare
|
I have to leave now, but hopefully I'll get back to this later in the day. Cheers. And thanks for the update... |
|
oh i did not notice you made commits. I had to rebase and resolve conflicts. I ran |
paulo-ferraz-oliveira
left a comment
paulo-ferraz-oliveira
left a comment
There was a problem hiding this comment.
When CI's passing, I'll merge this, then release. Thanks much.
src/setup-beam.js
Outdated
| } | ||
| versions = parseVersionFile(versionFilePath) | ||
|
|
||
| const versionFileType = getInput('version-file-type', '.tool-versions') |
There was a problem hiding this comment.
Tests are failing. I think we should not break the input name, but associate:
.tool-versions>asdfmise.toml>mise
What do you say?
There was a problem hiding this comment.
Can we be certain that the file name for current users will always be .tool-versions? Besides that, mise.toml can also be .mise.toml etc...
I wanted the change be non-breaking for current users so I introduced a new input to be explicit but default to .tool-versions for backwards compatibility. wdyt?
|
I should maybe add this: I see the PR got some love over the months. However, I have moved away from using this action towards using mise directly to install Elixir/Erlang on the CI (I've built my own re-usable workflows that I use in my projects). I think it's better that way as it's what you would do on your local env, too. |
|
|
||
| const versionFileType = getInput('version-file-type', '.tool-versions') | ||
| const versionFileType = | ||
| getInput('version-file-type', false) || '.tool-versions' |
There was a problem hiding this comment.
Per https://github.com/erlef/setup-beam/blob/main/action.yml#L71, not only is the input named differently, but there's no default.
There was a problem hiding this comment.
What I mean with this is:
- you should keep reading
version-file - you shouldn't need this new input, since you're already making for a
switchbelow, where you decide on the same values expected forversion-file
There was a problem hiding this comment.
- you should keep reading version-file
but I do keep reading version-file, no? I didn't change that part
- you shouldn't need this new input, since you're already making for a switch below, where you decide on the same values expected for version-file
My point is: We're not in control of what is expected for version-file. Mise currently supports more than 7 different forms of mise.toml. Not sure about what asdf supports besides .tool-versions
Or how would the switch look like in your version?
Thank you for this. I know of use cases where this'd be useful (I can't speak for the community at large, but when it was discussed before we were Ok to have this support in the action), which is being able to move from I understand you might not have time and/or want to deal with more of the PR, but lemme know and I can also take it to a mergeable state, in that case. |
Definitely something that's useful for my projects - thanks for all the work to get this ready! |
Feel free to take over or I can help - but might not be super responsive (after all, this was open without response for months). |
|
Closing in favor of #415. Thanks for all the initial work. |
3 participants
Description
Currently, only
.tool-versionsfiles types are supported asversion-file. This PR adds an inputversion-file-typewhich can be set tomise.toml(defaults to.tool-versions) in order to parse a mise-en-place config file.