harden github actions by perminder-17 · Pull Request #8650 · processing/p5.js

@perminder-17

Hardening github actions for dev-2.0

perminder-17

run: bash <(curl -s https://codecov.io/bash) -f coverage/coverage-final.json
env:
CI: true
uses: codecov/codecov-action@671740ac38dd9b0130fbe1cec585b89eea48d3de # v5.5.2

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

We're not really utilizing it at the moment so it probably can be skipped entirely. We'll do code coverage in Vitest for 2.x at some point and reporting can either use a service like this or even our own bot.

@perminder-17

ksen0

env:
CI: true
- name: Run test
run: npm test -- --project=unit-tests

Choose a reason for hiding this comment

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

Choose a reason for hiding this comment

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

Okay, I think I was directly using the same diff from the main branch which gets me to some unnecessary changes, Fixing that now.

ksen0

files: release/*
generate_release_notes: true
token: ${{ secrets.ACCESS_TOKEN }}
token: ${{ secrets.GITHUB_TOKEN }}

Choose a reason for hiding this comment

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

@perminder-17 I don't think the token should change (or, why should it?)

Choose a reason for hiding this comment

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

In Step 3 (Release p5.js), the release is created on the same repository, so GITHUB_TOKEN is sufficient. Unlike ACCESS_TOKEN, which is a long-lived Personal Access Token, GITHUB_TOKEN is automatically generated and scoped to the current repository for each workflow run, and expires once the workflow completes. ACCESS_TOKEN is only required in Step 4, where cross-repository access is needed to push changes to the p5.js-website repository. What you think?

Choose a reason for hiding this comment

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

Ah sure! Then GITHUB_TOKEN seems alright, thank you for the explanation

ksen0

env:
CI: true
- name: Run test
- name: Run build

Choose a reason for hiding this comment

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

Typo (?)

ksen0

env:
CI: true
- name: Run build
run: npm run build

Choose a reason for hiding this comment

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

also typo?

ksen0

with:
node-version: 22.x
persist-credentials: false
- name: Use Node.js 20.x

Choose a reason for hiding this comment

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

why 20?

ksen0

Choose a reason for hiding this comment

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

I left a few comments, thanks for working on this!

@perminder-17

I left a few comments, thanks for working on this!

I thought the diff for dev-2.0 would be the same as main, so I directly pushed the same changes there too. But it looks like a few unnecessary changes and maybe some typos got included in the 2.0 branch. Fixing that, :)