chore(ci): make cost-saving changes by aduh95 · Pull Request #8714 · nodejs/nodejs.org

chore(ci): make cost-saving changes by aduh95 · Pull Request #8714 · nodejs/nodejs.org

Description

IIUC we don't need full blown runners for most jobs, let's see if the slim ones do the job

Refs: https://docs.github.com/en/billing/reference/actions-runner-pricing

Validation

Related Issues

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run pnpm format to ensure the code follows the style guide.
I have run pnpm test to check if all tests are passing.
I have run pnpm build to check if the website builds without errors.
I've covered new added functionality with unit tests if necessary.

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/web-infra

Please review the changes when you have a chance. Thank you! 🙏

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.07%. Comparing base (ec16468) to head (552ed8b).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8714      +/-   ##
==========================================
- Coverage   75.10%   75.07%   -0.04%     
==========================================
  Files         104      104              
  Lines        9167     9167              
  Branches      315      316       +1     
==========================================
- Hits         6885     6882       -3     
- Misses       2280     2283       +3     
  Partials        2        2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Choose a reason for hiding this comment

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

Why has this been split into two separate jobs, rather than a matrix?

Choose a reason for hiding this comment

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

If you look at the commit history, you'll see I've started with that exact approach, and gave up as I wasn't able to make it work. Open to suggestions

Choose a reason for hiding this comment

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

Yes, it looks like the runs failed with Error when evaluating 'strategy' for job 'build'. .github/workflows/build.yml (Line: 41, Col: 21): Unexpected value 'pull_request', .github/workflows/build.yml (Line: 42, Col: 15): Unexpected value 'nodejs/nodejs.org', which seems like it was just a syntax error?

Choose a reason for hiding this comment

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

Like I said, open to suggestions?

Choose a reason for hiding this comment

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

I'd probably start with keeping it as the matrix it was before, and switching to the slim runner? Then if that works, look at adding the other changes you made that exclude certain runs etc.

Choose a reason for hiding this comment

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

Maybe that's just a me-thing, but your use of question mark on sentences that are clearly not questions is... frustrating. It reads to me as the equivalent of writing if(console.log('Hello')); instead of console.log('hello');.
We can discuss decoupling (and if someone wants to open an alternative PR, that's great), but the more pressing problems is that it looks like lots of jobs are not compatible with the slim runners.

Choose a reason for hiding this comment

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

They are questions. I am asking if that change can be done, as I would not like to land this as-is with the jobs being split out of what was a working matrix before, creating duplication.

Choose a reason for hiding this comment

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

Any update here?

Choose a reason for hiding this comment

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

I still need help to unblock the Error: EACCES: permission denied, open '/home/agent/post_event.json' failure

Choose a reason for hiding this comment

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

I'm a bit concerned about using the lightweight runners here, given we've struggled with long build times, and recently needed to switch to more powerful Vercel runners as well?