-
Notifications
You must be signed in to change notification settings - Fork 313
Fix custom hook output #197
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 custom hook output #197
Conversation
dblessing
commented
Nov 12, 2014
- Show stdout and stderr. Hooks might need to output info to user even if successful.
ffdc8f9
to
d7f77f6
Compare
end | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trailing whitespace detected.
d7f77f6
to
17e2c2c
Compare
@@ -22,7 +22,8 @@ def post_receive(changes, repo_path) | |||
def update(ref_name, old_value, new_value, repo_path) | |||
hook = hook_file('update', repo_path) | |||
return true if hook.nil? | |||
system(hook, ref_name, old_value, new_value) ? true : false | |||
system(hook, ref_name, old_value, new_value) | |||
$?.exitstatus == 0 ? true : false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why use global variable if system
returns valid value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, sorry. I misunderstood how system
works. Fixed.
17e2c2c
to
57c9080
Compare
@randx I hope this can slip in to a release for GitLab 7.5. Otherwise users could be confused if they're testing custom hooks |
@dblessing I doubt it but it is up to @jacobvosmaer |
FYI: current |
This is a nice improvement but I think it is not worth the hassle this close to the 7.5.0 release. |
Thanks @dblessing |