-
Notifications
You must be signed in to change notification settings - Fork 63
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
Should not exit with error when ghooks is missing #70
Comments
cc @dbaeumer |
+1 |
We could think about ways to make exiting with an error a configurable option (taken into account on |
Yeah, in retrospect, maybe this feature should have been released behind a flag so it doesn't break for people relying on the previous "default". On the other hand, and this can be very context specific, it could be potentially harmful to have On a separate note, but somewhat related to this issue, one thing that just occurred me is that if we embed |
Patching the logic into the hooks themselves sounds good for me. Updating ghooks as a dependency might then reinvoke the patching procedure to update the runner logic. |
I like it. This way the hooks themselves don't get too cluttered, with too many responsibilities. Going down this route, though, would have the hooks running regardless of @joaomoreno, @bruceCzK any 💭? |
Love that idea. |
I wanted to weigh in that this new behavior is really painful on a project where I've recently added ghooks, but developers are still working on branches that don't yet have it. For "reasons," they can't rebase on the branch that has ghooks installed, but if they've visited that ghooks branch and run npm install, then they have these unskippable hooks on their feature branch. I expected As a workaround, I've had these developers |
This should be fixed now. Could y'all try out the latest version and let us know please? Thank you for your patience :-) |
dcc01d2 introduced an error return value for when
ghooks
is not found.This is a really bad idea, since
git
commands start failing whenever thenode_modules
directory is not populated. This started breaking our (Microsoft/vscode) integration.I believe the previous behaviour of warning the user was the best one.
The text was updated successfully, but these errors were encountered: