-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[v5] breaking: make React 18 as minimal requirement #2236
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b4742b7:
|
env: | ||
DEVTOOLS_SKIP: ${{ matrix.devtools-skip }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are removing this, we can remove devtools-skip
entirely. Can you try that? devtools-skip
is a hack to pass CIs, so if it passes, it's fine.
Find also CI-MATRIX
in ./tests
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made that change to the best of my ability. I would appreciate a review of these changes because I'm not very familiar with the previous hacks in the CI although I understand why they were needed.
- devtools-skip: CI-MATRIX-NOSKIP | ||
build: umd | ||
build: [cjs, esm, umd] # [cjs, esm, umd, system] | ||
env: [development] # [development, production] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dai-shi, I copied this change from https://github.com/pmndrs/valtio/pull/819/files, but looking at this, could it be a mistake? I'd like to test both development
and production
in the testing matrix. Should this be changed to the following?
env: [development] # [development, production] | |
env: [development, production] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Related Issues or Discussions
Check List
yarn run prettier
for formatting code and docs