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.

Sign up for GitHub

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: add npm cache in actions/setup-node #4271

Merged
merged 8 commits into from
Dec 1, 2023
Merged

ci: add npm cache in actions/setup-node #4271

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35226ff
ci(opentelemetry-core): add npm cache in actions/setup-node@v3
lyzlisa Nov 8, 2023
6d4cd9e
Move checkout step before setup-node
lyzlisa Nov 10, 2023
bd7d3cb
Add name to checkout step in lint.yml for consistency across all actions
lyzlisa Nov 10, 2023
5da12f8
Bump actions/checkout to v4 in benchmark.yml
lyzlisa Nov 10, 2023
be4456b
Merge branch 'main' into gh-action-add-npm-ci
pichlermarc Nov 27, 2023
49b33f4
Merge branch 'main' into gh-action-add-npm-ci
trentm Nov 29, 2023
a085295
Merge branch 'main' into gh-action-add-npm-ci
trentm Nov 30, 2023
1fb2d00
Merge branch 'main' into gh-action-add-npm-ci
trentm Dec 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/benchmark.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,13 @@ jobs:
NPM_CONFIG_UNSAFE_PERM: true
steps:
- name: Checkout
uses: actions/checkout@v3
uses: actions/checkout@v4
Copy link
Contributor Author

Choose a reason for hiding this comment

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

May be out of scope for this PR, but all other workflows touched in this PR were already using v4. Let me know if it makes more sense to leave it out of this PR.

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to keep it in this PR👍
Most likely it was an oversight.


- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: ${{ matrix.node_version }}

- run: npm install -g npm@latest
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/docs.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: '18'

- name: Install and Build 🔧
Expand Down
7 changes: 5 additions & 2 deletions .github/workflows/lint.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,15 @@ jobs:
runs-on: ubuntu-latest

steps:
- name: Checkout
uses: actions/checkout@v4
- uses: actions/setup-node@v4
with:
cache: 'npm'
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious if we are able to notice any speed up in npm ci steps in this or other workflows as a result of caching. With the variable GitHub Action runners it may be hard to get reliable numbers, though.

Copy link
Member

@legendecas legendecas Nov 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

From what I can tell from a quick small number of checks at https://github.com/open-telemetry/opentelemetry-js/actions/workflows/unit-test.yml for node-test (v18), the run time reduced from 2m35s to about 2m with this PR.

cache-dependency-path: |
package-lock.json
Comment on lines +20 to +21
Copy link
Contributor

Choose a reason for hiding this comment

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

https://github.com/actions/setup-node#caching-global-packages-data suggests that package-lock.json will be used as a default:

The action defaults to search for the dependency file (package-lock.json, npm-shrinkwrap.json or yarn.lock) in the repository root, and uses its hash as a part of the cache key. Use cache-dependency-path for cases when multiple dependency files are used, or they are located in different subdirectories.

However, I think it is fine either way.

node-version: '16'

- uses: actions/checkout@v4

- name: Lint changelog file
uses: avto-dev/markdown-lint@v1
with:
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/unit-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: ${{ matrix.node_version }}

- run: npm install -g npm@latest
Expand Down Expand Up @@ -59,6 +62,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: '18'

- run: npm install -g npm@latest
Expand All @@ -85,6 +91,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: 16

- name: Bootstrap
Expand All @@ -106,6 +115,9 @@ jobs:
uses: actions/[email protected]
- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: 16

- name: Bootstrap
Expand Down Expand Up @@ -133,6 +145,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: ${{ matrix.node_version }}

- name: Build
Expand Down
3 changes: 3 additions & 0 deletions .github/workflows/w3c-integration-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ jobs:

- uses: actions/setup-node@v4
with:
cache: 'npm'
cache-dependency-path: |
package-lock.json
node-version: '16'

- name: Install and Bootstrap 🔧
Expand Down
Loading