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.

Sign up for GitHub

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

Jump to bottom

feat: add support for current and latest aliases #279

Closed
wants to merge 4 commits into from
Closed

feat: add support for current and latest aliases #279

wants to merge 4 commits into from

Conversation

theoludwig
Copy link

@theoludwig theoludwig commented Jul 1, 2021 •
edited

Fixes #257

The goal of this PR is to support current as alias for the node-version array.
I thought that current is a better name than latest because the actual name from Node.js is current but we can support both aliases if we really want to.

Currently, this is a Work In Progress, I don't know much of this codebase, so I would appreciate some help.
I'm new to the world of developing GitHub Actions, I always use them but never develop them 😆, I saw that you can test if it actually works with the tool nektos/act.
So I ran this command : act -j current-syntax and it seems to work, but it is not caching the Node.js version yet.

EDIT: I also added the latest alias.

@theoludwig theoludwig mentioned this pull request Jul 1, 2021
Closed
@theoludwig
Copy link
Author

cc @maxim-lobanov

@TomasHubelbauer
Copy link

I'd like to request that 'latest' also be supported, however even without it this would be great to have! Thank you for making the PR

@theoludwig theoludwig mentioned this pull request Jul 1, 2021
Closed
1 task
@theoludwig theoludwig changed the title feat: add support for current alias feat: add support for current and latest aliases Jul 1, 2021
@theoludwig
Copy link
Author

I'd like to request that 'latest' also be supported, however even without it this would be great to have! Thank you for making the PR

Done! 👍
I also added the latest alias in addition to the current one.

@Tungbengok
Copy link

Dowload ?

@theoludwig
Copy link
Author

From #278 (comment)

We will be ready to cut new action release at the begin of next week.

Could you review this PR and maybe include it in the next release if possible and everything is good to go, please ? 😄 @maxim-lobanov

Artek199 referenced this pull request Jul 26, 2021
@maxim-lobanov
Merge pull request #277 from actions/malob/update-readme
38d90ce 
Update readme to describe new NVM LTS syntax
@Kikobeats
Copy link

any chance to merge this?

@demurgos
Copy link

Hi,
It's been two months: is there any news about this PR?

@theoludwig
Copy link
Author

Thanks for your interest. @Kikobeats and @demurgos

I'm happy to continue to work on this PR if it's needed.
The main problem currently is that it is not caching the Node.js version, it's downloading again Node.js even if the current release hasn't changed.
That means it's slower than it should be, at least it works. 😄
But to set up caching, I would need some help from the maintainers.

@dmitry-shibanov
Copy link
Contributor

Hello @divlo. Thank you for your pull request. Could please attach link to the documentation for the current alias ? If I understood well, it should install the latest (lts/non lts) version ? My main concern is about that the latest alias is more obvious.

@theoludwig
Copy link
Author

Hello @divlo. Thank you for your pull request. Could please attach link to the documentation for the current alias ? If I understood well, it should install the latest (lts/non lts) version ? My main concern is about that the latest alias is more obvious.

Updated README and resolved the conflicts. 👍
Yes, it installs the latest Node.js version (for example currently it is v17.3.1).
The latest alias might be more obvious but the real name is current as you can see on the Node.js website.

image

@aivus
Copy link

aivus commented Jan 18, 2022 via email

@dominykas
Copy link

Just a memory dump of prior art:

I'd argue latest should not be supported explicitly.

@jehon
Copy link

jehon commented Jan 18, 2022

Could it be that the two would exists ? "current" and "latest" being synonyms ?

@dominykas
Copy link

My thinking is that "latest" can be misleading, esp. in the context of there being "latest in a release line", "latest LTS", etc. I'd rather have latest not work and error out, so that people do not pick that up and try to use it elsewhere, eventually leading to further confusion.

I could be wrong in my thinking, of course. I'll try to dig out some more discussion threads about this for reference.

@dominykas
Copy link

I guess a lot of threads can be followed from nodejs/package-maintenance#236

I do have to admit, that I have an issue open on nvm side to support latest myself, but ideally these keywords should all be converging...

@theoludwig
Copy link
Author

@dmitry-shibanov Friendly ping. 😄

What is needed to merge this PR?

@bluelovers
Copy link

Just a memory dump of prior art:

I'd argue latest should not be supported explicitly.

so does actions/setup-node support node too?

@bluelovers bluelovers mentioned this pull request Feb 16, 2022
Closed
@theoludwig
Copy link
Author

@bluelovers Does node alias is about the latest current Node.js released version (e.g: currently it is v17.5.0)?
If yes, that would solve this issue, but it just adds another naming...
What naming should we choose: current or latest or node or all the three together?

@bluelovers
Copy link

@bluelovers Does node alias is about the latest current Node.js released version (e.g: currently it is v17.5.0)? If yes, that would solve this issue, but it just adds another naming... What naming should we choose: current or latest or node or all the three together?

nvm , travis ci use node , and actions/setup-node support nvmrc, so i think node should be support

@jablko jablko mentioned this pull request Feb 17, 2022
Merged
5 tasks
@theoludwig
Copy link
Author

This PR has no recent activity, and probably won't be merged for a long time and as I don't want/have time to do the job for this PR, I decided to close it.

Feel free to open another PR yourself based on this one if you want this to be implemented.

@theoludwig theoludwig closed this Apr 20, 2022
@theoludwig theoludwig deleted the feat/current-alias branch April 20, 2022 12:38
deining pushed a commit to deining/setup-node that referenced this pull request Nov 9, 2023
@dependabot
chore(deps-dev): bump @vercel/ncc from 0.29.2 to 0.30.0 (actions#279)
c735be1 
Bumps [@vercel/ncc](https://github.com/vercel/ncc) from 0.29.2 to 0.30.0.
- [Release notes](https://github.com/vercel/ncc/releases)
- [Commits](vercel/ncc@0.29.2...0.30.0)

---
updated-dependencies:
- dependency-name: "@vercel/ncc"
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Artek199 Artek199 Artek199 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support latest as a version alias for the latest Node version