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

fix(deps): update husky package to the latest #403

Merged
merged 1 commit into from
Jan 12, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .husky/commit-msg
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

commitlint -E HUSKY_GIT_PARAMS
4 changes: 4 additions & 0 deletions .husky/pre-commit
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"

npm test
4 changes: 4 additions & 0 deletions .husky/pre-push
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/sh
Copy link
Contributor

Choose a reason for hiding this comment

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

While this is uncommon, some Windows systems are lacking sh. Is there a way to use Husky v7 for those?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I'm actually on Windows mostly and it works fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

Most Windows developers have sh installed, but not all.
Sometimes, an alternative is to use a Node script instead. However, I am wondering whether this is possible with Husky v7?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to bash, I don't have bash at all. The changes you see here are from their official migration.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also, these are git hooks, which work fine everywhere.

Copy link
Contributor Author

@XhmikosR XhmikosR Jan 10, 2022

Choose a reason for hiding this comment

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

So, basically husky sets the core.hooksPath of the git repo (when doing npm install) to be ./.husky, hence how this works :)

EDIT: git for Windows ships with its own MSYS2 system which includes bash, to answer your question about bash. So, anyone who's installed Git on Windows does have everything set up. I'm on Windows myself (mostly) and this works fine.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that most Windows developers have sh or Bash installed through git for Windows or other similar solutions. However, I have encountered some Windows developers who do not. While this is uncommon, I am wondering whether there is a way to make it work for them.

That being said, if Husky v7 does require sh, then this change sounds good to me.

What are your thoughts on this @erezrokah?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ehmicky please read my comments :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not most, everyone who installs Git on Windows has MSYS2 -> bash. That's how Git works on Windows. This is nothing new, it's like that forever.

So, what you are seeing here is simply git hooks. How they are handled it's outside of the scope of the PR but I explained that above :)

Copy link
Contributor

@ehmicky ehmicky Jan 10, 2022

Choose a reason for hiding this comment

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

Thanks @XhmikosR, I forgot that installing git implied installing Bash as well. You're absolutely correct.

. "$(dirname "$0")/_/husky.sh"

npm run format
8 changes: 1 addition & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ Linting and formatting configuration shared by Netlify Node.js repositories:
- [Prettier](https://prettier.io/)
- [Editorconfig](https://editorconfig.org/)
- `.gitattributes`
- `.husky/`

## How to add to a new Node.js repository

If you're creating a new repository, you can use the
[following GitHub template](https://github.com/netlify/node-template). Otherwise, please follow those steps:

- `npm install -D @netlify/eslint-config-node husky@4`
- Add a `.eslintrc.js` file to the root of the project. Based on the type of the project update the content of the file:

### Node.js project
Expand Down Expand Up @@ -87,12 +87,6 @@ module.exports = { extends: ['@commitlint/config-conventional'] }
"config": {
"eslint": "--ignore-path .gitignore --cache --format=codeframe --max-warnings=0 \"{src,scripts,tests,.github}/**/*.{js,md,html}\" \"*.{js,md,html}\" \".*.{js,md,html}\"",
"prettier": "--ignore-path .gitignore --loglevel=warn \"{src,scripts,tests,.github}/**/*.{js,md,yml,json,html}\" \"*.{js,yml,json,html}\" \".*.{js,yml,json,html}\" \"!package-lock.json\""
},
"husky": {
"hooks": {
"commit-msg": "commitlint -E HUSKY_GIT_PARAMS",
"pre-push": "npm run format"
}
}
}
```
Expand Down
Loading