FreeRTOS-Kernel
FreeRTOS-Kernel copied to clipboard
Request For Comment: Add additional PR checks for formatting
Add some additional linters from the pre-commit project.
Specifically enable these checks on files that have changed as part of a pull-request:
- trailing-whitespace (remove trailing whitespace from lines)
- end-of-file-fixer (check that files end with an LF or CRLF)
- check-yaml (link yaml files)
- check-added-large-files (really of less usefulness after a push)
- mixed-line-ending (reject files with a mix of LF and CRLF line endings)
- check-case-conflict (check files with the same name on case-insensitive filesystems)
- check-executables-have-shebangs
- check-json (link json files)
- check-merge-conflict (check for unresolved merge conflicts)
- check-symlinks (check of dangling symlinks)
- check-ast (validate python)
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
https://github.com/paulbartell/FreeRTOS-Kernel/pull/10 is an initial attempt to use uncrustify for formatting.
Not sure if a pre-commit hook is the best path forward, since that can be pretty annoying with unexpected formatting rewrites and having to constantly use --no-verify to suppress the hook. A better option might be to just catch this stuff in PR CI and provide an easy way for folks to run the autoformatter locally.
paulbartell#10 is an initial attempt to use uncrustify for formatting.
Not sure if a pre-commit hook is the best path forward, since that can be pretty annoying with unexpected formatting rewrites and having to constantly use
--no-verifyto suppress the hook. A better option might be to just catch this stuff in PR CI and provide an easy way for folks to run the autoformatter locally.
The uncrustify pre-commit hook needs some configuration so that it acts as one would expect. Namely, it needs to only run on files that have changed and operate in-place without leaving backup files everywhere. I'll do some debugging on this later this week.
The uncrustify pre-commit hook needs some configuration so that it acts as one would expect. Namely, it needs to only run on files that have changed and operate in-place without leaving backup files everywhere.
I believe that's how it currently operates by default (at least that's what I've observed in paulbartell#10 locally).
My point is that we may not want pre-commit to auto-edit files "behind the scenes" when a user simply wants to make a local commit.
The uncrustify pre-commit hook needs some configuration so that it acts as one would expect. Namely, it needs to only run on files that have changed and operate in-place without leaving backup files everywhere.
I believe that's how it currently operates by default (at least that's what I've observed in paulbartell#10 locally).
My point is that we may not want pre-commit to auto-edit files "behind the scenes" when a user simply wants to make a local commit.
For sure. It should be optional. You can always run "pre-commit" from within a git repo without installing it as a git pre-commit hook.
Codecov Report
Merging #274 (994bd14) into main (56428a9) will not change coverage. The diff coverage is
n/a.
@@ Coverage Diff @@
## main #274 +/- ##
=======================================
Coverage 92.13% 92.13%
=======================================
Files 4 4
Lines 1272 1272
Branches 342 342
=======================================
Hits 1172 1172
Misses 53 53
Partials 47 47
| Flag | Coverage Δ | |
|---|---|---|
| unittests | 92.13% <ø> (ø) |
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 56428a9...994bd14. Read the comment docs.