-
Notifications
You must be signed in to change notification settings - Fork 55
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
update pre-commit hooks #363
Conversation
Signed-off-by: Niclas Wesemann <niclas.wesemann@motius.de>
I am in general positive. Some things came to my mind.
|
MoM: Please Review |
Signed-off-by: Niclas Wesemann <niclas.wesemann@motius.de>
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 |
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 boschglobal#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 |
Signed-off-by: Niclas Wesemann <niclas.wesemann@motius.de>
MoM: Waiting for other PRs to be merged |
Would propose something like this #393 |
done by #393 |
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