-
Notifications
You must be signed in to change notification settings - Fork 82
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
Adopt ruff (or other linter) #193
Comments
I particularly like this idea. It would probably not be that useful for the |
Hi @alcides, The idea sounds interesting! To be honest, I've been wanting to use @folivetti, even though the changes to That said, creating a new CI for |
fair point, it would possibly make things stop passing the tests right now and it wouldn't be wise to do it at this moment. But I guess @alcides could do it and open a PR that we can merge after the benchmark is done (or maybe do it at a later moment if he's too busy right now). |
The main problem with the proposed change is that it will result in a huge commit that touches everything. This is aweful for people who are working on their branches/forks. My proposal is to do this (probably after the competition), when no-one is actively working on branches. It is not worthwhile to do it before hand. |
I agree!! We'll let you know when all the work in srbench seems to be finished. I'm looking forward to this (so I can "steal" to some other projects :-P ). Thanks @alcides ! |
Different contributors have different code styles. The recommended approach to handle this is to have a pre-commit/CI step that validates that the source code follows some style.
For Python, I have been using ruff, that automatically fixes many issues, and detects code smells.
If you think it's useful, I can prepare a PR. But I don't think I should spend the time if you do not think it's worthwhile.
The text was updated successfully, but these errors were encountered: