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
Fixes #79 - correctly pass future parser option #91
Conversation
@@ -68,7 +73,7 @@ for changedfile in $files_to_check; do | |||
failures=$((failures + 1)) | |||
fi | |||
elif [[ $(echo "$changedfile" | grep -q '\.*\.pp$'; echo $?) -eq 0 ]]; then | |||
${subhook_root}/puppet_manifest_syntax_check.sh "$changedfile" | |||
${subhook_root}/puppet_manifest_syntax_check.sh "$changedfile" "" "$USE_PUPPET_FUTURE_PARSER" |
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.
What's with the empty "" after $changedfile ?
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.
$2 is module dir (https://github.com/drwahl/puppet-git-hooks/blob/master/commit_hooks/puppet_manifest_syntax_check.sh)
but its not used with the pre-commit hook; only with the pre-receive hook
@@ -42,6 +42,11 @@ if [[ -f "$git_root/.git" ]]; then | |||
fi | |||
fi | |||
|
|||
# Only puppet 3.2.1 - 3.8 support "--parser future" option. | |||
case $(puppet --version) in | |||
4*) USE_PUPPET_FUTURE_PARSER="disabled" ;; |
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.
I would add some information about it to documentation, as now you will need to dig in code to find out, that you can enable it somehow.
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.
I did not add that feature, I just extended it to work in pre-commit hook aswell (previously it only worked in pre-receive hook); but I'll happyly add a short description in readme later today :)
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.
True, feature was quite undocumented, but cool that now it's there. Thanks for it!
for the pre-commit hook
should be issue #79 :)
Greetings
Klaas