-
Notifications
You must be signed in to change notification settings - Fork 15
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(setup): update Husky usage #199
Conversation
"husky-setup": "path-exists .husky/commit-msg || (husky init && pnpm husky-setup:commit-msg && pnpm husky-setup:post-merge && pnpm husky-setup:pre-commit)", | ||
"husky-setup:commit-msg": "echo 'pnpm run commit-msg' > .husky/commit-msg", | ||
"husky-setup:post-merge": "echo 'pnpm run setup' > .husky/post-merge", | ||
"husky-setup:pre-commit": "echo 'pnpm run pre-commit' > .husky/pre-commit", |
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.
Changes per Husky v9 changelog
"lint": "eslint index.js --fix", | ||
"lint:ci": "eslint index.js", | ||
"pre-commit": "pnpm lint && pnpm test", | ||
"prepare": "is-ci || pnpm husky-setup", | ||
"prepare": "husky", |
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.
Automatically updated by husky init
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! Besides the corepack
update. Looks like that causes CI to fail. What are thoughts?
e02ff80
to
7b402e9
Compare
@yowainwright I've removed the corepack config addition from this branch in order to make the Husky changes easily mergable. I'm going to poke at corepack solutions a bit since I'm curious about the best options for supporting both types of users. Without the entry, anything that triggers
(picking the current default version in my default mise node version apparently) We're in a weird limbo were people are starting to enable corepack as the default on their systems, but Node hasn't made it on by default quite yet more broadly. If there's a corepack config that works and you like, there's an interesting question of moving the source of the pnpm version in the setup script (for those not using corepack) to reading from package json, though reading JSON without |
Great work! |
Proposed Changes
Found while working on #197, pulled out to not force coupling.
Updates to using new Husky v9 commands
Deprecated
install
andadd
commands were causing lifecycle events to fail. Verified that hooks are still installed and functional after these changes.Adds
packageManager
entry topackage.json
to enable Corepack compatibilityReferences same version as referenced in setup/env. For Corepack users this will prompt automatic installation of the correct version, and will be noop for everyone else.