Skip to content

Upgrade NodeJS to 20.6.1 #54

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

Merged
merged 10 commits into from
Oct 25, 2023
Merged

Conversation

OneideLuizSchneider
Copy link
Contributor

No description provided.

@angelikatyborska
Copy link
Member

Hi! Thanks for bringing this to my attention. Node 16 had its end of life date two days ago 🙂

I noticed you're not updating the most important field that specifies the node version - the field runs.using in the action.yml. If I were to accept the PR as is, it would mean developing and testing the action against a different node version that is actually used to run it.

Of course the ideal course of action would be to update the node version everywhere, but I just learned that running Github actions on node18 is not supported, see: https://github.com/orgs/community/discussions/53217

Node 20 is supported by the Github action runner, and 20 will be the next LTS version, so we should just use that. Could you update your PR to use the latest node 20 version instead, also in the action.yml file?

@OneideLuizSchneider
Copy link
Contributor Author

Hi @angelikatyborska
I just upgraded it.

@OneideLuizSchneider OneideLuizSchneider changed the title Upgrade NodeJS to 18.17.1 Upgrade NodeJS to 20.6.1 Sep 13, 2023
@angelikatyborska
Copy link
Member

Thank you!

@angelikatyborska
Copy link
Member

Dependencies need to be upgraded to fix the npm build task, but there are some breaking changes that cause the action to break. I will continue testing and fixing it some other day.

@OneideLuizSchneider
Copy link
Contributor Author

OneideLuizSchneider commented Sep 13, 2023

cool, let me know if you need any help, I just saw the build command as well, I didn't run it before, I'd only tested lint and test.

@angelikatyborska
Copy link
Member

I managed to get everything working. Thanks again 🙏

@angelikatyborska angelikatyborska merged commit 9048315 into exercism:main Oct 25, 2023
@OneideLuizSchneider
Copy link
Contributor Author

Great, Thanks @angelikatyborska

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