Skip to content
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 the test for clang-format #660

Merged
merged 1 commit into from Apr 22, 2015
Merged

Fix the test for clang-format #660

merged 1 commit into from Apr 22, 2015

Conversation

bdesham
Copy link
Contributor

@bdesham bdesham commented Apr 21, 2015

This consists of two changes:

  1. The syntax in configure.ac was incorrect; the effect was that the build script always thought that the clang-format command was available, even if $CLANG_FORMAT was equal to no.
  2. format.sh did not notice if $CLANG_FORMAT was not an actual command; in this case it would actually report that the code was formatted correctly and exit with a 0 status code. (This happened because the result of clang-format was being piped through grep and so the failure code from the invalid clang-format command was silently ignored.)

This consists of two changes:

1. The syntax in `configure.ac` was incorrect; the effect was that the
   build script always thought that the `clang-format` command was
   available, even if `$CLANG_FORMAT` was equal to `no`.
2. `format.sh` did not notice if `$CLANG_FORMAT` was not an actual
   command; in this case it would actually report that the code was
   formatted correctly and exit with a 0 status code. (This happened
   because the result of `clang-format` was being piped through `grep`
   and so the failure code from the invalid `clang-format` command was
   silently ignored.)
@ggreer
Copy link
Owner

ggreer commented Apr 22, 2015

Thanks so much for finding and fixing these bugs.

ggreer added a commit that referenced this pull request Apr 22, 2015
@ggreer ggreer merged commit 722852c into ggreer:master Apr 22, 2015
@bdesham bdesham deleted the check-for-clang-format branch November 12, 2017 20:55
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.

None yet

2 participants