Skip to content
This repository was archived by the owner on May 15, 2025. It is now read-only.

Conversation

TheZoker
Copy link
Contributor

This PR adds a node installer module. It is powered by NVM. By default it installs the latest version of node and makes that the default. But the user can also provide a list of node versions, they like to install and also a custom default node version as well.

I'm not 100% sure about the naming. Other possible names for the module:

  • nvm
  • node
  • nodejs

What you you prefer?

Note: node installs the latest version of node.

@matifali matifali requested a review from mafredri February 18, 2024 06:10
@TheZoker
Copy link
Contributor Author

@mafredri @matifali Would appreciate, if you guys could have a look at this. Thanks!

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of comments, but a lot of them are the same suggestion repeated throughout. Hope it's not too overwhelming and thanks for creating this PR!

@TheZoker
Copy link
Contributor Author

TheZoker commented Mar 1, 2024

Thanks for the review! I think fixed all issue but one (see comment)

@TheZoker TheZoker requested review from mafredri and matifali March 1, 2024 14:17
@TheZoker TheZoker changed the title Add new node-via-nvm module Add new nodejs module Mar 1, 2024
@TheZoker
Copy link
Contributor Author

TheZoker commented Mar 1, 2024

Hmm very strange, I pushed changes to my branch and even rebased onto the latest main, but somehow github does not update this PR with the latest commit 😕
https://github.com/TheZoker/coder-modules/commits/add-nvm-module/

@mafredri Any idea why this could be?

@mafredri
Copy link
Member

mafredri commented Mar 1, 2024

@mafredri Any idea why this could be?

@TheZoker We'll have to wait, sit tight 😄 https://www.githubstatus.com/incidents/wcl1sw4mzg60

@TheZoker
Copy link
Contributor Author

TheZoker commented Mar 1, 2024

@mafredri Alright, seems like the Github sync job has caught up. Can you check if the script is ok now? Thanks!

Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor stuff, but this is mostly OK by me now. There's still the question of verified or not so I'll defer approval until that's been decided/taken care of.

Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
@TheZoker TheZoker requested a review from mafredri March 1, 2024 18:22
@TheZoker
Copy link
Contributor Author

TheZoker commented Mar 5, 2024

@mafredri Since we set verified to false for now, I think this issue should now be resolved, right?

@matifali matifali merged commit f335a62 into coder:main Mar 5, 2024
@TheZoker TheZoker deleted the add-nvm-module branch March 5, 2024 16:15
@TheZoker
Copy link
Contributor Author

TheZoker commented Mar 5, 2024

Thanks for the reviews and the merge 🎉

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants