-
Notifications
You must be signed in to change notification settings - Fork 8
Improve diagnostics from commit checker #5
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
Conversation
hooks/check-commits.sh
Outdated
@@ -26,8 +27,11 @@ trap finish EXIT | |||
hashes=$(git rev-list "$1") | |||
for h in ${hashes} | |||
do | |||
echo Checking $h... >&2 |
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.
Is there an option to only print this when there is an error?
All the check scripts stay silent when everything is ok.
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.
Maybe check-diff.py or check-message.py could output it themselves?
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 guess easiest is to pipe all output to another temporary file, and echo that to stderr at the end, if failure was detected.
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.
Seems marginally nasty. How about just outputting stuff on error, and checking for empty string?
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.
For me always displaying the commit being checked when there's no error is ok. And it would be even better to also display the commit message and not only the hash. Easier for contributors to identify where the error is.
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.
Make also prints the names of files being compiled as progress indicators. But the compiler still prefixes each message with the name of the file causing the message.
9ba2747
to
9a0fe9d
Compare
hooks/check-commits.sh
Outdated
|
||
finish() { | ||
rm -f ${tmp_msg_file} ${tmp_diff_file} | ||
} | ||
trap finish EXIT | ||
|
||
echo "::add-matcher::${HOOKS_DIR}/check-diff-matcher.json" |
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.
Do these screw with the actual githooks?
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 guess it will add a line to the output, but I don't know the effect of this line with the actual githooks (broken anyway see #11)
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.
Would it be possible to guard them with some env variable?
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.
Probably, using some GH action env variable. I'll see what I can do.
Instead of failing on the first problem, and then not identifying the commit that caused it, print commit ids as they are checked, and continue checking after finding problems. This should make it easier and faster to correct problems with commit messages and code formatting.
According to https://help.github.com/en/actions/configuring-and-managing-workflows/using-environment-variables#default-environment-variables I think matchers are now guarded |
Instead of failing on the first problem, and then not identifying the commit that caused it, print commit ids as they are checked, and continue checking after finding problems. This should make it easier and faster to correct problems with commit messages and code formatting.