Skip to content

Add CI steps and bump node version#43

Merged
peternandersson merged 17 commits intomainfrom
peter/add-ci
Mar 25, 2026
Merged

Add CI steps and bump node version#43
peternandersson merged 17 commits intomainfrom
peter/add-ci

Conversation

@peternandersson
Copy link
Copy Markdown
Contributor

@peternandersson peternandersson commented Mar 24, 2026

This PR does a few things:

  • Adds a CI step for build, lint, & format for each PR & post-merge
  • Bumps the node version to 24 because we can bump actions to ones that support that version

Fixes a couple of exposed issues:

  • The yarn lock after another PR merged a package json change without installing
  • Lint throwing warnings instead of errors by default

peerDependencies:
react: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0
react-dom: ^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0
peerDependenciesMeta:
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I missed this change in a previous PR. CI correctly caught it.

@peternandersson peternandersson changed the title Add CI for build, lint, & format Add CI steps and bump node version Mar 24, 2026
@peternandersson peternandersson marked this pull request as ready for review March 24, 2026 21:48
@peternandersson peternandersson requested a review from a team March 24, 2026 21:49
run: corepack enable

- name: Setup Node.js
uses: actions/setup-node@6044e13b5dc448c55e2357c09f80417699197238 # v6
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I believe node_modules caching is built into this action. Can we leverage that instead of trying to handle the caching ourselves?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh good find. I'll push & play around with it

@peternandersson peternandersson requested review from a team and abeljohn March 25, 2026 23:07
cache: 'yarn'

- name: Install dependencies
run: yarn install ${{ inputs.immutable-install == 'true' && '--immutable' || '' }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yarn install is immutable by default in CI. I don't mind adding the explicit flag, but I don't think there's any case where we don't want the install to be immutable in CI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That's surprising. Does yarn throw a status code of 1 when yarn install updates the lock file?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes

@peternandersson peternandersson merged commit da232ee into main Mar 25, 2026
4 checks passed
@peternandersson peternandersson deleted the peter/add-ci branch March 25, 2026 23:25
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.

2 participants