-
-
Notifications
You must be signed in to change notification settings - Fork 8.1k
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
chore: upgrade to Husky 7 #6304
Conversation
- Upgrade to Husky 7 - Update install hook to "prepare" - Remove parallel/CI packages
Hi @nschonni! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
✔️ [V2] 🔨 Explore the source changes: 12f4ee1 🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61dbdeee5f62a400070303a7 😎 Browse the preview: https://deploy-preview-6304--docusaurus-2.netlify.app |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-6304--docusaurus-2.netlify.app/ |
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.
Thank you! As soon as the CLA check passes we can merge.
I signed it, so the bot is probably just sneaker-netting it somewhere |
I have a question: last time I investigated, it seems Husky is now installed for CI. Compare: I don't think it's very problematic though, since AFAIK we never commit during CI |
Yeah, it can take a while for the CLA check to come through, no worries😉 |
Looks like they do still talk about it in https://typicode.github.io/husky/#/?id=disable-husky-in-cidocker. I thought it fully wasn't needed because they had dropped the old env variables to skip the install. |
I have no idea why we had that check in the first place since we don't git commit in CI, but I'm fine with removing it since it probably only saves a few seconds of installation. Any value you can see in not installing for CI? |
Not really. I could see it maybe tripping up on repos that use semantic-relaese plugins and commitlint, but I don't think it should matter here |
Sure, let's go with this way👍 The check was added when we did the v4 -> v5 bump. |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Okay, let's go! |
Motivation
Newer husky changed the need moved the post-install to
prepare
script, so a few other packages could now be droppedHave you read the Contributing Guidelines on pull requests?
(Write your answer here.)
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Related PRs
(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)