setup(ci): improve #39
Conversation
| @@ -8,8 +8,9 @@ jobs: | |||
|
|
|||
| steps: | |||
| - uses: actions/checkout@v4 | |||
| - uses: ljharb/actions/node/install@main | |||
| name: 'nvm install lts/* && npm install' | |||
| - uses: actions/setup-node@v4 | |||
There was a problem hiding this comment.
this is not a preferred change, it's actually less simple, and it's not maintained by me so tc39 has less control over it.
There was a problem hiding this comment.
It's the simplest because everyone uses it, because it manages versions for us, and it hides the npm dep.
There was a problem hiding this comment.
my bad I mean cache.
But FIY "cache" in French mean hide. So just ADHD
There was a problem hiding this comment.
my actions can do caching as well, but caching adds complexity that can hide bugs, so it's usually best avoided when speed isn't a priority, as it isn't here.
There was a problem hiding this comment.
just realize that caching can be accomplish without lock file
There was a problem hiding this comment.
Let's get back to the main topic, which is that action/setup-node is used by everyone and is very well maintained.
And I remain convinced that using a standard GHA is the right approach.
There was a problem hiding this comment.
It is not always well maintained - there's been problems with it before - and it doesn't really matter who else uses it. We're going to keep using the same things we've been using across the tc39 org, they work well and we can directly fix issues in them, unlike in setup-node.
| - run: npm run build | ||
| cache: 'npm' | ||
| - run: npm ci | ||
| - run: node --run build |
There was a problem hiding this comment.
we can't use node --run, because it won't run pre and post scripts.
There was a problem hiding this comment.
You should never use these side effects to perform a task.
if you want a second step explicitly add it
There was a problem hiding this comment.
That's an opinion, and one i profoundly disagree with.
There was a problem hiding this comment.
In most cases, it seems obvious to me that having each step clearly written down instead of relying on magic is the best idea.
There was a problem hiding this comment.
it is clearly written down, in package.json's scripts section. This is similar to how makefiles have operated for decades, it's not magic, it's what's expected.
There was a problem hiding this comment.
Yes, it's just that it's something voluntary. But I don't think we should base our decision on that. Because a task, a script, a step in the CI, a call to a script...
|
CI time is very fast as it is, and doesn't need improvement really. |
This reverts commit 83a66b7.
|
jordan had right, again ... 😁 |
Description
actions/setup-node@v4lts/*--runsince LTS isv22.x