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

Skip a hook based on the GITHOOKS_SKIP / _DISABLE env vars #44

Merged
merged 1 commit into from Mar 7, 2017

Conversation

Projects
None yet
3 participants
@choroba
Contributor

choroba commented Feb 27, 2017

Fixes #6.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 27, 2017

Coverage Status

Coverage increased (+0.08%) to 90.065% when pulling 221b949 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

coveralls commented Feb 27, 2017

Coverage Status

Coverage increased (+0.08%) to 90.065% when pulling 221b949 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

Show outdated Hide outdated lib/App/GitHooks.pm
{
my ( $name ) = @_;
return unless exists $ENV{'GITHOOKS_SKIP'};

This comment has been minimized.

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

Based on Tim's comment, I think it would be worth adding (in addition to GITHOOKS_SKIP) a GITHOOKS_DISABLE env variable that skips all hooks without having to name them one by one.

Also, we should document all those environment variables in a centralized place - I'd suggest adding a new =head1 around https://github.com/guillaumeaubert/App-GitHooks/blob/master/lib/App/GitHooks.pm#L399.

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

Based on Tim's comment, I think it would be worth adding (in addition to GITHOOKS_SKIP) a GITHOOKS_DISABLE env variable that skips all hooks without having to name them one by one.

Also, we should document all those environment variables in a centralized place - I'd suggest adding a new =head1 around https://github.com/guillaumeaubert/App-GitHooks/blob/master/lib/App/GitHooks.pm#L399.

Show outdated Hide outdated lib/App/GitHooks.pm
@@ -452,6 +452,8 @@ sub run
croak "Invalid hook name $name"
if scalar( grep { $_ eq $name } @$HOOK_NAMES ) == 0;
return $HOOK_EXIT_SUCCESS if _should_skip( $name );

This comment has been minimized.

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

We may want to print a warning on STDERR here, to indicate that hooks were intentionally skipped. This will be a good reminder in case someone "inadvertently" adds this to their .bash_profile.

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

We may want to print a warning on STDERR here, to indicate that hooks were intentionally skipped. This will be a good reminder in case someone "inadvertently" adds this to their .bash_profile.

This comment has been minimized.

@choroba

choroba Feb 28, 2017

Contributor

Yes. We should also create a follow-up ticket to add a note to the commit message if the pre-commit hook was skipped.

@choroba

choroba Feb 28, 2017

Contributor

Yes. We should also create a follow-up ticket to add a note to the commit message if the pre-commit hook was skipped.

@@ -0,0 +1,58 @@
#!perl

This comment has been minimized.

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

I ❤️ tests. Thank you!

@guillaumeaubert

guillaumeaubert Feb 28, 2017

Owner

I ❤️ tests. Thank you!

@choroba

This comment has been minimized.

Show comment
Hide comment
@choroba

choroba Feb 28, 2017

Contributor

OK, here's a fixup. Let me know if it's OK, I'll squash the commits then.

Contributor

choroba commented Feb 28, 2017

OK, here's a fixup. Let me know if it's OK, I'll squash the commits then.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Feb 28, 2017

Coverage Status

Coverage increased (+0.1%) to 90.103% when pulling e934183 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

coveralls commented Feb 28, 2017

Coverage Status

Coverage increased (+0.1%) to 90.103% when pulling e934183 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

@guillaumeaubert

This comment has been minimized.

Show comment
Hide comment
@guillaumeaubert

guillaumeaubert Mar 1, 2017

Owner

Looks great - thank you for the tweaks and extra tests! I'll merge after you squash the commits then.

Are you planning to do other PRs? Just checking to see if I should hold off a release on CPAN.

Owner

guillaumeaubert commented Mar 1, 2017

Looks great - thank you for the tweaks and extra tests! I'll merge after you squash the commits then.

Are you planning to do other PRs? Just checking to see if I should hold off a release on CPAN.

@choroba

This comment has been minimized.

Show comment
Hide comment
@choroba

choroba Mar 1, 2017

Contributor

Squashed. As February's over, I'm moving to a next task :-) Thanks for the help.

Contributor

choroba commented Mar 1, 2017

Squashed. As February's over, I'm moving to a next task :-) Thanks for the help.

@coveralls

This comment has been minimized.

Show comment
Hide comment
@coveralls

coveralls Mar 1, 2017

Coverage Status

Coverage increased (+0.1%) to 90.103% when pulling 9783911 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

coveralls commented Mar 1, 2017

Coverage Status

Coverage increased (+0.1%) to 90.103% when pulling 9783911 on choroba:i6-env-skip into d94a7b8 on guillaumeaubert:master.

@choroba choroba changed the title from Skip a hook based on the GITHOOKS_SKIP env var to Skip a hook based on the GITHOOKS_SKIP / _SIDABLE env vars Mar 1, 2017

@choroba choroba changed the title from Skip a hook based on the GITHOOKS_SKIP / _SIDABLE env vars to Skip a hook based on the GITHOOKS_SKIP / _DISABLE env vars Mar 1, 2017

@guillaumeaubert guillaumeaubert merged commit 1399c6d into guillaumeaubert:master Mar 7, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@guillaumeaubert

This comment has been minimized.

Show comment
Hide comment
@guillaumeaubert

guillaumeaubert Mar 7, 2017

Owner

Thank you so much again, @choroba! Merged, and I'll do a release shortly with your changes.

Owner

guillaumeaubert commented Mar 7, 2017

Thank you so much again, @choroba! Merged, and I'll do a release shortly with your changes.

@choroba choroba deleted the choroba:i6-env-skip branch May 12, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment