Install all command #32
Conversation
|
Wow, that is nice!! I have some little advices, I will do them in the commits. |
|
Cool, sounds good |
|
On the installer, the ruby_path is nil by default, but in the template we set a new default (ruby_path || some default path). This default path should go to installer default para, doesn't? |
| @@ -17,7 +17,6 @@ | |||
| require_relative 'git_hooks/pre_commit' | |||
|
|
|||
| module GitHooks | |||
| HOOK_SAMPLE_FILE = 'hook.sample' | |||
| HOOKS = [PRE_COMMIT = 'pre-commit'] | |||
There was a problem hiding this comment.
I don't think this was used? I might be incorrect
|
I see, yeah that makes sense |
| .open(hook_path, 'w') | ||
| .write(hook_script) | ||
|
|
||
| FileUtils.chmod(0775, hook_path) |
There was a problem hiding this comment.
I think this is to make the hook an executable? @rranelli may be able to speak to this better
|
That sounds promissor! The a lot. |
|
@stupied4ever please let me know which changes you'd like still. I've commented on the lines you've mentioned. I still would like to write tests for this |
|
|
||
| hook_script = ERB.new(hook_template).result(binding) | ||
|
|
||
| puts "Writing to file #{hook_path}" |
There was a problem hiding this comment.
This needs to be fixed as you included logs on other PR
|
Probably the only thing you need to fix is the puts (as you created the log feature). |
|
Gotcha, I'll merge upstream when I get home |
|
other point I was thinking is that we got a Install All command, but by the moment we have only PreCommit hooks. We created A multi hook type but we did not completed other hooks Type. The user will never be able to install all, only install PreCommit. I need to find somre time to complete that PR that still in WIP |
1 similar comment
Still needs automated tests. Tested manually
git_hooks installto install every hookhook.sampleinto an ERB template and generate the correct template based on the hook