-
Notifications
You must be signed in to change notification settings - Fork 24
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
Does not work with GUI's (e.g. smartGit) #8
Comments
Seems the full path to node is needed. And, even maybe to grunt. |
Interesting, didn't know about the different behaviours with GUIs. I'd like the plugin to work with GUIs too, so I'll look into a solution for this. I made a quick test in the issue-8 branch to put the full path to Grunt in the hook (obtained from the argv array). If you want to give it a try, you can replace the
In the meanwhile, I've updated the README to specify the use cases the plugin is still struggling with :) |
Well, I think that is one of the issues. The next issue is my git GUI is choking on the node environment, guess because its not the correct path to node either. |
Quick try to put the node path in the hook too, it's on the same issue-8 branch. You might need to clean npm cache for grunt-githooks (
If you want to make sure you have the right version, the files in the Does that solve the problem with the NodeJS path ? |
might be fast if you quickly tested it with http://www.syntevo.com/smartgithg/ |
Indeed :) Was just trying some quick fixes before stopping work on the project for the evening. I'll have a deeper look at it, hopefully later this week, and test it properly with a couple of different GUIs to see if they all behave the same regarding the paths to Node and Grunt. |
I took a bit more time to investigate the issue, and test the behaviour of the hooks with various GUI. The hooks worked fine with SmartGit 4.6.4 on Windows and 4.6.2 on Ubuntu. Is this the version you're using? If it is a Mac OS X specific issue, I will have trouble investigating it as I don't have a Mac. Could you check it is indeed a matter of providing the full path to NodeJS and Grunt? You can manually edit the hook file in the If this is the case, I think I'm going to change the way to address the issue. I don't think it's the plugin's responsibility to insert full paths in the generated hooks to overcome shortcomings of a 3rd party GUI on a specific platform. The plugin should however provide the necessary options to make things works with this GUI. So I'm thinking of a new option to let people define the path to Grunt. This will let the devs decide if they need the full path or not, according to the tools they use around Git. It will also let them decide of the way to get this path (hardcoded in the Gruntfile, detected from process.argv, other...). And as a bonus, they'll be able to hook other commands if they want. What do you think ? I think it'd also be useful to raise a bug to SmartGit so that their GUI make hooks run in a similar fashion as when run with the Git CLI. |
FWIW, I struggled with this for a bit. Here's the config that ended up working for me, based on stuff i found in the README:
|
Seems that it should be stressed that this might not work for developer's that make use of a GIT GUI. It's usage assumes the command line. Or am I wrong? Issue I see is this forces all developers working on the code to work only with GIT commands, on the command line. I bounce back and forth, because conflicts and logs are faster for me in smartGit. But this breaks commits in smartGit.
On the other hand, could this not be updated to deal with git GUI's?
http://stackoverflow.com/questions/17538460/using-git-pre-commit-hooks-in-context-of-github-client
The text was updated successfully, but these errors were encountered: