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

[fei5403.1.workflowstonode20] Move workflows to node 20 #2144

Merged
merged 4 commits into from
Dec 19, 2023

Conversation

somewhatabstract
Copy link
Member

Summary:

This updates the GitHub workflows to use Node 20 instead of 16.

Issue: FEI-5403

Test plan:

I verified that with Node 20, I could install node modules and also run a variety of commands:

  • yarn test
    • Some snapshots needed to be updated due to a change in the error thrown by JSON.parse. Instead of SyntaxError: Unexpected token B in JSON at position 0, the error looks like SyntaxError: Unexpected token 'B', "BAD JSON" is not valid JSON
  • yarn typecheck
  • yarn lint

@somewhatabstract somewhatabstract self-assigned this Dec 15, 2023
@somewhatabstract somewhatabstract requested a review from a team December 15, 2023 00:15
Copy link

changeset-bot bot commented Dec 15, 2023

🦋 Changeset detected

Latest commit: 60f8bbd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 0 packages

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@khan-actions-bot khan-actions-bot requested a review from a team December 15, 2023 00:15
@khan-actions-bot
Copy link
Contributor

khan-actions-bot commented Dec 15, 2023

Gerald

Required Reviewers
  • @Khan/wonder-blocks for changes to .changeset/quiet-snakes-report.md, .github/workflows/chromatic-build.yml, .github/workflows/chromatic-pr.yml, .github/workflows/deploy.yml, .github/workflows/gerald-push.yml, .github/workflows/node-ci-pr.yml, .github/workflows/node-ci-push.yml, .github/workflows/release.yml, packages/wonder-blocks-testing/src/__tests__/respond-with.test.ts, packages/wonder-blocks-data/src/util/__tests__/get-gql-data-from-response.test.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

Copy link
Contributor

github-actions bot commented Dec 15, 2023

npm Snapshot: Published

🎉 Good news!! We've packaged up the latest commit from this PR (a11829c) and published all packages with changesets to npm.

You can install the packages in webapp by running:

./services/static/dev/tools/deploy_wonder_blocks.js --tag="PR2144"

Packages can also be installed manually by running:

yarn add @khanacademy/wonder-blocks-<package-name>@PR2144

Copy link
Contributor

github-actions bot commented Dec 15, 2023

Size Change: +273 B (0%)

Total Size: 91.6 kB

Filename Size Change
packages/wonder-blocks-modal/dist/es/index.js 5.32 kB +273 B (+5%) 🔍
ℹ️ View Unchanged
Filename Size
packages/wonder-blocks-accordion/dist/es/index.js 3.69 kB
packages/wonder-blocks-banner/dist/es/index.js 1.53 kB
packages/wonder-blocks-birthday-picker/dist/es/index.js 1.72 kB
packages/wonder-blocks-breadcrumbs/dist/es/index.js 1.13 kB
packages/wonder-blocks-button/dist/es/index.js 4.27 kB
packages/wonder-blocks-cell/dist/es/index.js 2.24 kB
packages/wonder-blocks-clickable/dist/es/index.js 3.24 kB
packages/wonder-blocks-color/dist/es/index.js 1.15 kB
packages/wonder-blocks-core/dist/es/index.js 3.7 kB
packages/wonder-blocks-data/dist/es/index.js 6.33 kB
packages/wonder-blocks-dropdown/dist/es/index.js 12.3 kB
packages/wonder-blocks-form/dist/es/index.js 5.34 kB
packages/wonder-blocks-grid/dist/es/index.js 1.36 kB
packages/wonder-blocks-i18n/dist/es/index.js 4.54 kB
packages/wonder-blocks-icon-button/dist/es/index.js 3.21 kB
packages/wonder-blocks-icon/dist/es/index.js 1.06 kB
packages/wonder-blocks-labeled-field/dist/es/index.js 72 B
packages/wonder-blocks-layout/dist/es/index.js 1.88 kB
packages/wonder-blocks-link/dist/es/index.js 2.54 kB
packages/wonder-blocks-pill/dist/es/index.js 1.03 kB
packages/wonder-blocks-popover/dist/es/index.js 4.38 kB
packages/wonder-blocks-progress-spinner/dist/es/index.js 1.51 kB
packages/wonder-blocks-search-field/dist/es/index.js 1.55 kB
packages/wonder-blocks-spacing/dist/es/index.js 158 B
packages/wonder-blocks-switch/dist/es/index.js 2.06 kB
packages/wonder-blocks-testing/dist/es/index.js 3.94 kB
packages/wonder-blocks-theming/dist/es/index.js 1.21 kB
packages/wonder-blocks-timing/dist/es/index.js 1.78 kB
packages/wonder-blocks-toolbar/dist/es/index.js 862 B
packages/wonder-blocks-tooltip/dist/es/index.js 5.05 kB
packages/wonder-blocks-typography/dist/es/index.js 1.49 kB

compressed-size-action

Copy link
Contributor

github-actions bot commented Dec 15, 2023

A new build was pushed to Chromatic! 🚀

https://5e1bf4b385e3fb0020b7073c-lqlmetqade.chromatic.com/

Chromatic results:

Metric Total
Captured snapshots 324
Tests with visual changes 0
Total stories 402
Inherited (not captured) snapshots [TurboSnap] 0
Tests on the build 324

Copy link

codecov bot commented Dec 15, 2023

Codecov Report

Merging #2144 (60f8bbd) into main (3029c9f) will decrease coverage by 1.51%.
Report is 1 commits behind head on main.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2144      +/-   ##
==========================================
- Coverage   97.06%   95.55%   -1.51%     
==========================================
  Files         242      244       +2     
  Lines       28199    28256      +57     
  Branches     2459     2318     -141     
==========================================
- Hits        27371    27000     -371     
- Misses        828     1256     +428     

see 33 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3029c9f...60f8bbd. Read the comment docs.

Copy link
Member

@jeresig jeresig left a comment

Choose a reason for hiding this comment

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

How'd this drop our code coverage by 1%??

@somewhatabstract
Copy link
Member Author

How'd this drop our code coverage by 1%??

I have no idea. Very curious. If we're using Node's built-in coverage tracking, maybe that changed. I can investigate after the main migration of we need to

@somewhatabstract
Copy link
Member Author

somewhatabstract commented Dec 15, 2023

@jeresig

// Provides more accurate coverage reports by instrumenting more lines
// than the default provider.
coverageProvider: "v8",
// Only output log messages on test failure. From:

We're using the v8 code coverage and I expect there have been some significant changes to that between v16 and v20 of Node. That probably accounts for the change in coverage stats.

Node 20 is the first stable release of this feature.

@jeresig
Copy link
Member

jeresig commented Dec 15, 2023

@somewhatabstract Ahh, ok - that's a bummer!

@somewhatabstract somewhatabstract merged commit 636a716 into main Dec 19, 2023
12 of 13 checks passed
@somewhatabstract somewhatabstract deleted the fei5403.1.workflowstonode20 branch December 19, 2023 18:36
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.

3 participants