Support LTS aliases

[gordey4doronin]

The work is related to both #26 and #32.

For manifest changes see actions/versions-package-tools#32 and actions/node-versions#63.

• Support lts/codename aliases
• Support lts/* alias
• Test coverage for lts/codename aliases
• Test coverage for lts/* alias
• Test coverage for malformed aliases
• Clarify whether I need to commit compiled dist files or not? CC @maxim-lobanov
• Discuss whether forcing check-latest = true is good idea or not? CC @maxim-lobanov
• Discuss whether it's worth adding support for simple erbium or lts/erbium is enough? CC @maxim-lobanov @konradpabjan @bryanmacfarlane

The goal of my PR is to support .nvmrc syntax.
Given: .nvmrc/.node-version file in a repository
When: run setup-node action and pass content of the file to version property
Then: the action can recognize the format and choose correct node version

Being said that, from my point of view supporting simple erbium is not required, since version manager doesn't allow that.
Also, having all codenames starting with lts/ makes it really easy to determine in the code when lts alias passed to the action.

---

Reading the .nvmrc file is not the goal of this PR.
A separate PR shall be created for that, but it wouldn't be possible to implement .nvmrc reading without supporting the format.
So, think about current PR as a pre-requisite. 🙂

[maxim-lobanov]

@gordey4doronin , could you please also add e2e test. See example: https://github.com/actions/setup-node/blob/main/.github/workflows/versions.yml#L32
I think adding new job lts-syntax with node-version: ["lts/*", "lts/Fermium"] should be enough.

P.S. I believe we also need to update docs but we will take care about it in separate PR since we will rework docs in scope of #272

[gordey4doronin]

It seems some mocks are lacking for windows test. Will fix later.

[gordey4doronin]

could you please also add e2e test.

Absolutely. Good point. Will add too 👍

[gordey4doronin]

• Fixed unit tests for windows (was just a path resolution problem)
• Added e2e tests for lts syntax
• Unfortunately had to remove __tests__/verify-node.sh step from it, since the check script doesn't allow to parse lts syntax. However, I it should be enough to have only the main step, it's running actions/checkout@v2 and verifying lts alias resolution. If it couldn't resolve lts alias - it will fail. Profit.

[Kikobeats]

this is a super great functionality that has been not implemented until now, even this package is used for a lot of maintainers.

@maxim-lobanov @konradpabjan can you merge this, please? 🙏

[maxim-lobanov]

@gordey4doronin @Kikobeats , Merged PR. I will cut new version later today. Need to merge one more PR before that.

[JimiC]

Good to see that it only took you about 2 years to finally implement "basic" functionality. Let's hope we, now, can use this action in our CI/CD.

P.S. Sorry if I sound cynical but 2 years is a loooong time for such a vital CI/CD component.

[skjnldsv]

Thank you everyone that worked on this! Great addition! 🎉 🤗 ❤️

[JimiC]

@skjnldsv What you don't know and that's why you reacted with 👎 is that I was the first to submit a PR with the exact same functionality 2 YEARS ago.

[skjnldsv]

@JimiC sorry, I understand your frustration, but I don't see how your message helps anyone nor benefit the community in any sort! 😕

Have a great day! ☀️

[JimiC]

@JimiC sorry, I understand your frustration, but I don't see how your message helps anyone nor benefit the community in any sort! 😕

@skjnldsv It's just that when GH actions went live I was so excited about it and one of the first to try it out. So much that I even tried to contribute to the community and the community turned their back on me. So they made me from a GH fan to a GitLab fan.

Have a great day! ☀️

You too. ☮️

[mrlubos]

@gordey4doronin Can you help me to clarify my understanding? Can I at this point remove the node-version argument from the action setup and it will be able to infer the correct version from my .nvmrc file? If not, why? Would it be possible to add this functionality?

[gordey4doronin]

@mrlubos Unfortunately you can't.👇 I wish that too. 🙏

Reading the .nvmrc file is not the goal of this PR.
A separate PR shall be created for that

"Resolving aliases" and "reading version manager file" are two different technical tasks.
They're related to each other from business PoV.
But they're not related from code/implementation PoV.

My goal in the first place was to add LTS aliases resolving.

Now it's possible to use this workaround if you have aliases in your .nvmrc file (which I do).

Reading the .nvmrc file is a standalone task, and was out of scope of my completely voluntarily PR.

For reading the file I personally up-voted #32 and actions/runner#1180. 🙂

Hope that makes sense.

[mrlubos]

@mrlubos Unfortunately you can't.👇 I wish that too. 🙏

Reading the .nvmrc file is not the goal of this PR.
A separate PR shall be created for that

"Resolving aliases" and "reading version manager file" are two different technical tasks.
They're related to each other from business PoV.
But they're not related from code/implementation PoV.

My goal in the first place was to add LTS aliases resolving.

Now it's possible to use this workaround if you have aliases in your .nvmrc file (which I do).

Reading the .nvmrc file is a standalone task, and was out of scope of my completely voluntarily PR.

For reading the file I personally up-voted #32 and actions/runner#1180. 🙂

Hope that makes sense.

Thanks @gordey4doronin! I checked the issue you linked and what do you know, my vote is already there, too 😃 Have a nice day!