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: Clean up Node.js installation in GitHub actions #5322

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

watson
Copy link
Collaborator

@watson watson commented Feb 26, 2025

What does this PR do?

Clean up Node.js installation in GitHub actions

  • Remove actions installing old Node.js versions no longer used (14 and 16)
  • Rename latest action to active-lts to clearly signal intent (a separate latest or current action that tests non-lts, can be created in a follow up commit)
  • Remove 18 action, and replace any usage of it with oldest-maintenance-lts
  • Remove setup action and replace any usage of it with active-lts
  • Move install action to after the first Node.js install action that actually needs it, instead of first installing one Node.js version, only to install another right after
  • Change any direct usage of actions/setup-node to use our own Node.js installation actions instead, when a matrix is not used.
  • A few places where Node.js 18 was used for testing, now use active-lts as this was most likely the intent (👉 reviewers should verify! 👈 )

Motivation

  • Align how Node.js is installed in our GitHub actions
  • Reduce runtime of GitHub actions by not running unnecessary steps
  • Make it easier to upgrade to newer major Node.js versions when they are released (with the use of labels like active-lts instead of hardcoded version numbers)

@watson watson self-assigned this Feb 26, 2025
Copy link
Collaborator Author

watson commented Feb 26, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

Copy link

github-actions bot commented Feb 26, 2025

Overall package size

Self size: 8.81 MB
Deduped: 95.01 MB
No deduping: 95.53 MB

Dependency sizes | name | version | self size | total size | |------|---------|-----------|------------| | @datadog/libdatadog | 0.4.0 | 29.44 MB | 29.44 MB | | @datadog/native-appsec | 8.4.0 | 19.25 MB | 19.26 MB | | @datadog/native-iast-taint-tracking | 3.3.0 | 13.77 MB | 13.78 MB | | @datadog/pprof | 5.5.1 | 9.79 MB | 10.17 MB | | protobufjs | 7.2.5 | 2.77 MB | 5.16 MB | | @datadog/native-iast-rewriter | 2.8.0 | 2.6 MB | 2.74 MB | | @opentelemetry/core | 1.14.0 | 872.87 kB | 1.47 MB | | @datadog/native-metrics | 3.1.0 | 1.06 MB | 1.46 MB | | @opentelemetry/api | 1.8.0 | 1.21 MB | 1.21 MB | | import-in-the-middle | 1.11.2 | 112.74 kB | 835.4 kB | | source-map | 0.7.4 | 226 kB | 226 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | lru-cache | 7.18.3 | 133.92 kB | 133.92 kB | | pprof-format | 2.1.0 | 111.69 kB | 111.69 kB | | @datadog/sketches-js | 2.1.0 | 109.9 kB | 109.9 kB | | lodash.sortby | 4.7.0 | 75.76 kB | 75.76 kB | | ignore | 5.3.2 | 53.63 kB | 53.63 kB | | shell-quote | 1.8.1 | 44.96 kB | 44.96 kB | | istanbul-lib-coverage | 3.2.0 | 29.34 kB | 29.34 kB | | rfdc | 1.3.1 | 25.21 kB | 25.21 kB | | @isaacs/ttlcache | 1.4.1 | 25.2 kB | 25.2 kB | | tlhunter-sorted-set | 0.1.0 | 24.94 kB | 24.94 kB | | limiter | 1.1.5 | 23.17 kB | 23.17 kB | | dc-polyfill | 0.1.4 | 23.1 kB | 23.1 kB | | retry | 0.13.1 | 18.85 kB | 18.85 kB | | semifies | 1.0.0 | 15.84 kB | 15.84 kB | | jest-docblock | 29.7.0 | 8.99 kB | 12.76 kB | | crypto-randomuuid | 1.0.0 | 11.18 kB | 11.18 kB | | ttl-set | 1.0.0 | 4.61 kB | 9.69 kB | | path-to-regexp | 0.1.12 | 6.6 kB | 6.6 kB | | koalas | 1.0.2 | 6.47 kB | 6.47 kB | | module-details-from-path | 1.0.3 | 4.47 kB | 4.47 kB |

🤖 This report was automatically generated by heaviest-objects-in-the-universe

Copy link

codecov bot commented Feb 26, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.44%. Comparing base (8cd547e) to head (fc07a2b).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5322      +/-   ##
==========================================
+ Coverage   80.42%   80.44%   +0.01%     
==========================================
  Files         491      492       +1     
  Lines       21847    21879      +32     
==========================================
+ Hits        17570    17600      +30     
- Misses       4277     4279       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

- Remove actions installing old Node.js versions no longer used (`14`
  and `16`)
- Rename `latest` action to `active-lts` to clearly signal intent (a
  separate `latest` or `current` action that tests non-lts, can be
  created in a follow up commit)
- Remove `18` action, and replace any usage of it with
  `oldest-maintenance-lts`
- Remove `setup` action and replace any usage of it with `active-lts`
- Move `install` action to after the first Node.js install action that
  actually needs it, instead of first installing one Node.js version,
  only to install another right after
- Change any direct usage of `actions/setup-node` to use our own Node.js
  installation actions instead, when a matrix is not used.
@watson watson force-pushed the watson/clean-up-gh-action-node-install branch from b229cfc to fc07a2b Compare February 26, 2025 11:58
@datadog-datadog-prod-us1
Copy link

datadog-datadog-prod-us1 bot commented Feb 26, 2025

Datadog Report

Branch report: watson/clean-up-gh-action-node-install
Commit report: 7804145
Test service: dd-trace-js-integration-tests

✅ 0 Failed, 673 Passed, 0 Skipped, 16m 34.62s Total Time

@pr-commenter
Copy link

pr-commenter bot commented Feb 26, 2025

Benchmarks

Benchmark execution time: 2025-02-26 13:05:25

Comparing candidate commit fc07a2b in PR branch watson/clean-up-gh-action-node-install with baseline commit 8cd547e in branch master.

Found 0 performance improvements and 0 performance regressions! Performance is the same for 917 metrics, 16 unstable metrics.

Copy link
Collaborator

@BridgeAR BridgeAR left a comment

Choose a reason for hiding this comment

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

LGTM, this is IMO a great improvement over the status quo

@BridgeAR BridgeAR merged commit 2070869 into master Feb 26, 2025
698 checks passed
@BridgeAR BridgeAR deleted the watson/clean-up-gh-action-node-install branch February 26, 2025 14:15
watson added a commit that referenced this pull request Feb 27, 2025
- Remove actions installing old Node.js versions no longer used (`14`
  and `16`)
- Rename `latest` action to `active-lts` to clearly signal intent (a
  separate `latest` or `current` action that tests non-lts, can be
  created in a follow up commit)
- Remove `18` action, and replace any usage of it with
  `oldest-maintenance-lts`
- Remove `setup` action and replace any usage of it with `active-lts`
- Move `install` action to after the first Node.js install action that
  actually needs it, instead of first installing one Node.js version,
  only to install another right after
- Change any direct usage of `actions/setup-node` to use our own Node.js
  installation actions instead, when a matrix is not used.
watson added a commit that referenced this pull request Feb 27, 2025
- Remove actions installing old Node.js versions no longer used (`14`
  and `16`)
- Rename `latest` action to `active-lts` to clearly signal intent (a
  separate `latest` or `current` action that tests non-lts, can be
  created in a follow up commit)
- Remove `18` action, and replace any usage of it with
  `oldest-maintenance-lts`
- Remove `setup` action and replace any usage of it with `active-lts`
- Move `install` action to after the first Node.js install action that
  actually needs it, instead of first installing one Node.js version,
  only to install another right after
- Change any direct usage of `actions/setup-node` to use our own Node.js
  installation actions instead, when a matrix is not used.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants