COVESA
update pre-commit hooks
Hi everyone,
as mentioned in issue #341, in my opinion it makes sense to use more pre-commit hooks to enforce proper formatting, correct spelling, and so on.. in all files that will be changed from now on. It will be a bit more difficult to pass the pipeline but it will also increase code quality.
This is still open for discussion though. I am happy to receive any comments
I am in general positive. Some things came to my mind.
- Should we possibly document why we introduce the respective hooks. In the commit, in the PR or possibly in the file? Like, why do we add ruff when we already have flake8. Not saying that it is bad, maybe there are benefits having both, but possibly also a risk for conflicts if they have different rules.
- Have you done some tests on how much this will change files, i.e. how much errors/changes will you get if trying to do a random edit in one of our larger Python files? I.e. does it seem feasible to update existing code whenever someone touches a file, or should we better handle the "bigger" files as a separate update activity to make sure they comply with the rules.
Sry for the late reply.. According to this blog post it is best to use ruff in pre-commit only for python >=3.11, so I removed it for now. But this is still open for discussion, does it make sense to add more modules to our pre-commit config?
Sry for the late reply.. According to this blog post it is best to use ruff in pre-commit only for python >=3.11, so I removed it for now. But this is still open for discussion, does it make sense to add more modules to our pre-commit config?
I read the blog post somewhat differently, that it now supports 3.11 (but also older version)
Until February 2023, Ruff lacked support for pattern matching, a feature introduced in Python 3.10, which was a showstopper for many considering Ruff adoption. Luckily, that got fixed (relevant issue) and currently Ruff claims to be compatible with 3.11
The homepage states support for 3.7-3.13
hi @erikbosch, you are right. I was referring to this paragraph and now I also read it differently. I would suggest we switch to ruff as it is much faster (written in rust) and find a configuration that matches the current one. For flake8 we only use the max_line_length = 120 which we could easily apply in the pyproject.toml (which is one way to configure ruff)
Feel free to revert to ruff. Concern config, I assume this change may have some minor effects on #367 as it mentions flake8 as dev dependencies. I did by the way do some tests on both flake8 and ruff at https://github.com/boschglobal/vss-tools/pull/5. Looks good as I see it. But maybe we should merge #367 first as changing pre-commit will introduce quite a lot of changes (and thus merge conflicts)
converting this back to draft for the moment as I can't get it to work the way I want, will revisit asap
