Skip to content

Conversation

@temideewan
Copy link
Contributor

Introduced a preinstall hook that ensures that husky is installed after a npm install.
Checks for proper commit messages should pass properly after this.

@JamieSlome
Copy link
Member

@temideewan - I'm just thinking, does it make sense to run this before every npm install. Although, that said, is there any other point in time that we can run this?

@temideewan
Copy link
Contributor Author

temideewan commented Jul 1, 2023

I see what you're saying.
All the options I've considered so far have to do with making sure that husky install actually runs before any interaction with the project.
The reasonable thing anyone would run before interacting with the project (especially when they pull an update) is npm install that's why I'm trying to attach it to one of the lifecycle hooks.
One thing I don't know yet is how to check if the current user has already ran husky install.

According to this blog we could switch preinstall: "npx husky install" for prepare: "husky install" and it'd still achieve the same end results.
But prepare also runs with npm install so it's a bit dicy to figure out a good balance

@JamieSlome
Copy link
Member

@temideewan - thank you for doing the deep dive! Happy to go with the hook you think is most appropriate.

Happy to proceed? I'll merge once you give me the green flag!

Copy link
Member

@JamieSlome JamieSlome left a comment

Choose a reason for hiding this comment

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

LGTM! 🍰

@JamieSlome JamieSlome merged commit 7fb860c into finos:main Jul 1, 2023
@temideewan
Copy link
Contributor Author

Yes there's an improvement I actually found to ensure we don't run it every single time, but I don't have access to my system right now so I can't test it out right now.
If you still feel a bit uncomfortable with the husky install running every single time this blog gives a pretty good description of the solution I wanted to try out.
Maybe you could have a look at it.

@JamieSlome
Copy link
Member

Yes there's an improvement I actually found to ensure we don't run it every single time, but I don't have access to my system right now so I can't test it out right now. If you still feel a bit uncomfortable with the husky install running every single time this blog gives a pretty good description of the solution I wanted to try out. Maybe you could have a look at it.

Sorry, I didn't get back to you! I didn't have the time to get around to this over the weekend. Do you think we could give this another look? I have seen the usage of prepare in different projects and it seems to work. Shall we create an issue?

@temideewan
Copy link
Contributor Author

Yes there's an improvement I actually found to ensure we don't run it every single time, but I don't have access to my system right now so I can't test it out right now. If you still feel a bit uncomfortable with the husky install running every single time this blog gives a pretty good description of the solution I wanted to try out. Maybe you could have a look at it.

Sorry, I didn't get back to you! I didn't have the time to get around to this over the weekend. Do you think we could give this another look? I have seen the usage of prepare in different projects and it seems to work. Shall we create an issue?
Yes I think we should

@JamieSlome
Copy link
Member

@temideewan - created #188 👍

coopernetes pushed a commit to coopernetes/git-proxy that referenced this pull request Oct 13, 2023
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.

2 participants