-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
chore: use npx --yes #7549
chore: use npx --yes #7549
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
@@ -0,0 +1 @@ | |||
# Docusaurus core |
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 didn't realize the core has no README... We should definitely have something better...
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.
yes 😄 also surprised
this was just for the CI anyway
@Josh-Cena the CI path filtering is a bit too aggressive IMHO When we commit to root package.json or other similar global things, all CIs should rather run? In my case only a few checks did run, and I had to commit a readme to trigger more. While I really want Windows CI to run in this case because it was the reason we didn't merge the previous PR We probably want to include a few extra things:
Any opinion? |
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
Agree to trigger for package.json & jest config, but probably not yarn lock, because lockfile changes without package JSON changes are... not meant to be there, if you get what I mean. They do not really mean anything in practice. |
Size Change: 0 B Total Size: 798 kB ℹ️ View Unchanged
|
Not sure what you mean here If you decide to re-generate a lockfile without touching package.json file, and due to upgraded dependencies the build fails on Windows for some obscure reason, don't we want to catch it before merging? |
Ah yes, that would make sense. |
Motivation
temp workaround for npm/cli#4619 until we use a newer node/npm version in CI matrix without this bug
Already attempted in #7167 : we can now try again thanks to Node 16 upgrade