Skip to content
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: review commit hooks #1345

Merged
merged 4 commits into from
Jan 13, 2025
Merged

chore: review commit hooks #1345

merged 4 commits into from
Jan 13, 2025

Conversation

stephenkilbourn
Copy link
Contributor

@stephenkilbourn stephenkilbourn commented Dec 19, 2024

Related Ticket: Close #1329

Description of Changes

It seems that there are some changes in both Husky and Yarn in how to make them play nicely together. Husky needs to be run once for each user before the package begins enforcing the git hooks. When it does that, you should see a new directory .husky/_. For npm this is done through a prepare script. Yarn doesn't support this, so Husky recommends a postinstall script. https://typicode.github.io/husky/how-to.html#manual-setup

I've added that script, as well as added a pre-push hook to run the type check and unit tests. That's easy to remove if contributors prefer pushing separate commits to fix/add tests, though.

Copy link

netlify bot commented Dec 19, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit ec6601f
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/676464ac071ce700080e63b7
😎 Deploy Preview https://deploy-preview-1345--veda-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@AliceR AliceR left a comment

Choose a reason for hiding this comment

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

Thank you @stephenkilbourn ! Both pre-commit and pre-push hook are working for me now 🎉

This will also close #1263 !

@stephenkilbourn stephenkilbourn merged commit 929f03c into main Jan 13, 2025
11 checks passed
@stephenkilbourn stephenkilbourn deleted the 1329-review-commit-hooks branch January 13, 2025 16:08
@@ -25,6 +25,7 @@
"lint": "yarn lint:scripts && yarn lint:css",
"lint:scripts": "eslint app/scripts/",
"lint:css": "stylelint 'app/styles/**/**' 'app/scripts/**/*.(js|ts|tsx|jsx)'",
"postinstall": "husky",
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will run after the install of veda-ui as a registry, which we don't want it. Do we need this @stephenkilbourn

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hanbyul-here - ah, I didn't realize that was going to be a consideration. Looking further at the husky docs for the setup of yarn, we need that postinstall setup, but we also need to disable it before and after the npm publish setup:

{
  "scripts": {
    // Yarn doesn't support prepare script
    "postinstall": "husky",
    // Include this if publishing to npmjs.com
    "prepack": "pinst --disable",
    "postpack": "pinst --enable"
  }
}

i'm adding those in a follow-up PR here: #1384

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.

Review commit hooks
3 participants