Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ci: updated github actions ci workflow #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Phillip9587
Copy link
Contributor

The same has been done for body-parser expressjs/body-parser#546 and router pillarjs/router#143

I noticed that the current CI workflow in this repository could benefit from some updates. Specifically:

  1. Node.js Installation in the Matrix Test Run:

    • Currently, the workflow uses nvm to install Node.js versions.
    • It might be worth considering switching to the official actions/setup-node action. This action can leverage cached Node.js versions from the runner, which could improve efficiency and speed up the CI pipeline.
  2. Deprecation of Artifact Actions v3:

    • The actions/upload-artifact@v3 and actions/download-artifact@v3 actions are being deprecated as of November 30, 2024 (GitHub Deprecation Notice).
    • These actions should be updated to their latest versions to ensure continued functionality in the CI workflow.
  3. The Coverage setup could also be optimized:

    • Currently, the workflow uses the coverallsapp/github-action@master which points to v1 of this action. This v1 action uses node16 as runtime which is deprecated.
  4. Minimum token permissions for the GITHUB_TOKEN:

Copy link

@bjohansebas bjohansebas left a comment

Choose a reason for hiding this comment

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

I would suggest making the linter a separate job, as it is done in express.

https://github.com/expressjs/express/blob/52ed64606fc1f5114d90265a66275a18f2d773af/.github/workflows/ci.yml#L23-L41

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

Successfully merging this pull request may close these issues.

2 participants