Skip to content

Conversation

@AugustinMauroy
Copy link
Member

Description

  • Cache dep
  • use node --run (stable on node 22)

Related Issues

No related issues

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I've covered new added functionality with unit tests if necessary.

@AugustinMauroy AugustinMauroy requested a review from a team as a code owner March 1, 2025 20:54
@AugustinMauroy AugustinMauroy requested a review from Copilot March 1, 2025 20:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

PR Overview

This PR updates the CI workflow to utilize caching for npm dependencies and switches script commands from "npm run" to "node --run" for better stability on Node 22.

  • Adds caching for npm via actions/setup-node.
  • Replaces "npm run" commands with "node --run" for linting, formatting, and test coverage.
  • Removes the standalone test step in favor of the coverage-enhanced test step.

Reviewed Changes

File Description
.github/workflows/pr.yml Updated workflow to implement caching and replace npm run commands with node --run equivalents

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

.github/workflows/pr.yml:27

  • Ensure that caching using 'cache: "npm"' is supported by the pinned version of actions/setup-node and behaves as expected with Node 22.
cache: 'npm'

.github/workflows/pr.yml:36

  • Verify that 'node --run format' correctly triggers the formatting script, ensuring consistency with the previous command.
run: node --run format

.github/workflows/pr.yml:39

  • Ensure that 'node --run test:coverage' covers the same test cases and behavior as the prior 'npm run test:coverage' command.
run: node --run test:coverage

Copy link
Member

@canerakdas canerakdas left a comment

Choose a reason for hiding this comment

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

LGTM

@AugustinMauroy AugustinMauroy merged commit adefb11 into main Mar 2, 2025
7 checks passed
@AugustinMauroy AugustinMauroy deleted the upgrade-ci branch March 2, 2025 12:31
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.

4 participants