Skip to content

Conversation

@getsnoopy
Copy link
Contributor

Some git hooks can be passed additional information through
standard input (e.g., the pre-push hook), so this commit handles
that properly. Since stream data can be consumed only once but
multiple hooks may need to use it, capture the stream data once
and pipe it to each child process individually.

Git hooks can be passed multiple arguments when they're called
(e.g., the pre-push hook gets passed the name and location of
the destination remote). The hooks were only being passed the 2nd
argument, so this commit fixes that.
Some git hooks can be passed additional information through
standard input (e.g., the pre-push hook), so this commit handles
that properly. Since stream data can be consumed only once but
multiple hooks may need to use it, capture the stream data once
and pipe it to each child process individually.
@coveralls
Copy link

coveralls commented Sep 5, 2016

Coverage Status

Coverage decreased (-0.9%) to 94.286% when pulling 8146487 on getsnoopy:master into 166b569 on tarmolov:master.

@tarmolov
Copy link
Owner

tarmolov commented Sep 7, 2016

Node 0.10.x does not support the keywords "inherit" as individual
values when defining child process file descriptors, so define
them explicitly.
@coveralls
Copy link

coveralls commented Sep 9, 2016

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 8ea4514 on getsnoopy:master into 166b569 on tarmolov:master.

lib/git-hooks.js Outdated
*
* @param {String} filename Path to git hook.
* @param {String[]} [args] Git hook arguments.
* @param {String} input Input string to be passed into each hook's standard input.
Copy link
Owner

Choose a reason for hiding this comment

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

Please do not align jsdoc :)
It's harder to maintain (see code complete book).

@tarmolov
Copy link
Owner

It's better to add an extra test to check that input is passed properly to the hook.

@coveralls
Copy link

coveralls commented Sep 26, 2016

Coverage Status

Coverage decreased (-0.9%) to 94.286% when pulling 31edbf3 on getsnoopy:master into 166b569 on tarmolov:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 3687cfb on getsnoopy:master into 166b569 on tarmolov:master.

4 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 3687cfb on getsnoopy:master into 166b569 on tarmolov:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 3687cfb on getsnoopy:master into 166b569 on tarmolov:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 3687cfb on getsnoopy:master into 166b569 on tarmolov:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) to 95.238% when pulling 3687cfb on getsnoopy:master into 166b569 on tarmolov:master.

@tarmolov tarmolov merged commit 74c8549 into tarmolov:master Sep 26, 2016
@tarmolov
Copy link
Owner

Thank you for you help!

Published version 1.1.2.

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.

3 participants