[llvm] [openmp] [polly] GitHub Actions: Lint Python code for just for SyntaxErrors (PR #123940)
Mats Jun Larsen via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 23 08:13:27 PST 2025
https://github.com/junlarsen commented:
> If _**that line of code**_ runs... In `summarizeStats.py` line 40 will only crash on the end user if line 39 evaluates to True and line 146 will only crash if line 145 evaluates to True. This means that this file can be run repeatedly without detecting these syntax errors but then with a certain input or when a certain function is called they crash in production. These runtime crashes are quite different from and more dangerous than compile-time crashes.
I think the fact that it hasn't been changed yet is a fair indication of how much use the script gets, but in any case I don't see any harm in updating it.
> If we can find 4 files in one second then coverage is incomplete and we should not sweep runtime crashes under the rug.
Same as above, I don't think they get enough use to warrant a new linter, but I don't see a problem in updating them.
> I would encourage you to use `pre-commit` in a smaller project to see the just-in-time value that it delivers.
We have a git plugin for clang-format and encourage developers to configure hook it up as a pre-push hook themselves https://llvm.org/docs/Contributing.html#git-pre-push-hook
>From my personal experience pre-commit hooks can do just as much harm as good if they're not done right, and I just don't think there is enough Python code in the monorepo that sees enough use by non-automation to warrant a pre-commit hook or new infrastructure.
There's also thousands of developers working on LLVM, so enforcing any commit hooks should be done with great consideration
https://github.com/llvm/llvm-project/pull/123940
More information about the llvm-commits
mailing list