clinicjs / node-clinic

Clinic.js diagnoses your Node.js performance issues
https://clinicjs.org
MIT License
5.67k stars 125 forks source link

chore: Use cache in `ci.yml` #377

Closed jongwooo closed 1 year ago

jongwooo commented 1 year ago

Signed-off-by: jongwooo jongwooo.han@gmail.com

Details

Updated ci.yml to cache dependencies using actions/setup-node. setup-node@v3 or newer has caching built-in.

AS-IS

- name: Install Node.js 18
  uses: actions/setup-node@v3
  with:
    node-version: 18

- name: Install dependencies
  run: npm install

TO-BE

- name: Install Node.js 18
  uses: actions/setup-node@v3
  with:
    node-version: 18
    cache: npm

- name: Install dependencies
  run: npm ci

It’s literally a one line change to pass the cache: npm input parameter.

References

simoneb commented 1 year ago

@jongwooo as you can see the workflows are failing because npm ci only works with a lockfile, which we don't have in this repo. @RafaelGSS any thoughts? Shall we add it so we can also enable this optimization or are we happy without?

jongwooo commented 1 year ago

@simoneb Oh, I forgot that there was no lockfile...!

jongwooo commented 1 year ago

@jongwooo as you can see the workflows are failing because npm ci only works with a lockfile, which we don't have in this repo. @RafaelGSS any thoughts? Shall we add it so we can also enable this optimization or are we happy without?

If there is no plan to add a lockfile, I will close this PR.

RafaelGSS commented 1 year ago

Usually, we don't use lockfiles on npm modules.

jongwooo commented 1 year ago

@simoneb Thank you for your careful comment! And @RafaelGSS, duly noted. Thank you! I will close this PR.

RafaelGSS commented 1 year ago

Just to expand on that, see: https://github.com/sindresorhus/ama/issues/479#issuecomment-310661514.