12 node version issues

[micaherne]

This fixes #12

It's basically removing the NODE_VERSION environment variable that was preventing nvm from installing the correct version from Moodle's .nvmrc file.

I tested it and it is working. I also checked that it doesn't just happen to be working on account of Moodle 4.3 having the latest LTS version in its .nvmrc at the moment. I set the nvm install in the Dockerfile to use an ancient version (lts/carbon) as the default, which is used to install grunt-cli, and it still worked (moodle-plugin-ci itself used nvm to install and enable lts/iron)

[NeillM]

I tried to run this branch against one of our plugins and when setting the tests to be for Moodle 4.1 and Moodle 4.3 it seemed to use nvm 20 both times, or is the npm version here for moodle-plugin-ci, rather than Moodle? If it is for the ci plugin then I guess we can ignore the Moodle .nvmrc file and have fixed versions of npm.

I'm guessing that is because we are using the default branch (which from what I have seen seems to be a fixed number and not necessarily take a .nvmrc file into account)

The runs did both appear to work though.

Having looked at things more I wonder if we should move the setting up of the node version to be inside moodlePluginCiInstall, since I guess that is where we actually have Moodle code in place.

[micaherne]

Thanks for testing it! I think what you're seeing there is probably to do with the .nvmrc file being updated to the same in both Moodle 4.1 and 4.3, which seems to have happened fairly recently: https://github.com/moodle/moodle/blob/MOODLE_401_STABLE/.nvmrc

That's a good idea about setting up node in moodlePluginCiInstall - it would certainly be a lot easier. I think we'd still need to have some version of npm available at the point the moodle-plugin-ci project is created by composer though, as it uses it in a post install script: https://github.com/moodlehq/moodle-plugin-ci/blob/4.4.0/composer.json#L111

I've been looking at the github actions script and I can't for the life of me figure out where nvm is actually installed there, so I'm wondering if we might be able just to remove all the nvm stuff somehow. I'll see if I can find out before merging this.

[NeillM]

Well doh! I guess I should update my development Moodle

I could imagine npm might be used in linting (assuming they use grunt for that)

[micaherne]

I've found it! The base images they're using in Github actions have various tools installed, including nvm, npm and nodejs: https://github.com/actions/runner-images/blob/main/images/ubuntu/Ubuntu2204-Readme.md

It also appears to install grunt locally: https://github.com/actions/runner-images/blob/main/images/ubuntu/scripts/build/install-nodejs.sh

Presumably if we were just to copy what they're doing there it would work. So the image would have specific versions of nvm and grunt-cli, and moodle-plugin-ci would handle installing the correct npm / nodejs with nvm.

[NeillM]

And here is the code in moodle-plugin-ci that installs node.js

So yes it does look like it may well not be needed here, as long as it is possible for moodle-plugin-ci to do the install, which seems to just require that nvm is installed

[micaherne]

That's making sense now! So I think we're at the position where we need:

• any version of npm at the point the create-project command is run
• nvm and any version of grunt-cli available at the point the install command is run (specifically the "Install npm dependencies" bit)

I'll tweak the patch to do that (and probably just go with the preinstalled version numbers from the github runner image)