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(check-formatting.sh): use relative directory #2

Closed
wants to merge 1 commit into from

Conversation

ahans
Copy link
Owner

@ahans ahans commented Mar 4, 2024

The bin/check-formatting.sh script changed its directory to bin as the first step, consequently expecting paths of files to check to be relative to bin, when typically they would be given relative to the working directory. When not finding a file, the script would just print a warning, but still exit with error code 0, which is probably the reason why this went unnoticed for quite a while.

This commit fixes the relative directory issue. Further, when a given path cannot be checked, we return a non-zero code now. Lastly, the script would count the number of files with formatting issues and return that number as error code. This has the issue that if the number of violating files is a multiple of 256, error code would falsly be returned.

The `bin/check-formatting.sh` script changed its directory to `bin` as
the first step, consequently expecting paths of files to check to be
relative to `bin`, when typically they would be given relative to the
working directory. When not finding a file, the script would just print
a warning, but still exit with error code 0, which is probably the
reason why this went unnoticed for quite a while.

This commit fixes the relative directory issue. Further, when a given
path cannot be checked, we return a non-zero code now. Lastly, the
script would count the number of files with formatting issues and return
that number as error code. This has the issue that if the number of
violating files is a multiple of 256, error code would falsly be returned.
@ahans ahans closed this Mar 15, 2024
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.

1 participant