Remove pylint and flake8, replacing with ruff #1290

2023-04-17T14:28:28Z

itamarst commented

2023-04-17 14:28:28 +00:00

(Migrated from github.com)

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/4014

As a follow-up we should probably start enabling a lot more checks. I estimate running every single check on CI would take maybe 4 seconds (on my computer it's 300ms due to multiple cores and faster CPU). This would create a bunch of noise so we don't want everything, but there are many more checks that would be useful to have.

Impacts on codechecks runtime in CI:

Switching to ruff shaved a minute off.
Doing skip_install = true in tox.ini shaved off another 20 seconds.

Fixes https://tahoe-lafs.org/trac/tahoe-lafs/ticket/4014 As a follow-up we should probably start enabling a lot more checks. I estimate running _every single check_ on CI would take maybe 4 seconds (on my computer it's 300ms due to multiple cores and faster CPU). This would create a bunch of noise so we don't want everything, but there are many more checks that would be useful to have. Impacts on `codechecks` runtime in CI: * Switching to ruff shaved a minute off. * Doing `skip_install = true` in `tox.ini` shaved off another 20 seconds.

exarkun (Migrated from github.com) approved these changes 2023-04-21 12:54:45 +00:00

exarkun (Migrated from github.com) left a comment

Thanks. One comment inline. Please address and merge.

tox.ini

exarkun (Migrated from github.com) commented

2023-04-21 12:54:22 +00:00

The version in setup.py is pinned precisely but this one is unpinned.

It's not even entirely obvious to me why we have this (or why we had pyflakes) as a dependency in setup.py. So devs could get the tools with pip install ...[test] I guess? It would be nice to declare this only once and (I guess?) for the declaration to include a version pin.

The version in setup.py is pinned precisely but this one is unpinned. It's not even entirely obvious to me why we have this (or why we had pyflakes) as a dependency in setup.py. So devs could get the tools with `pip install ...[test]` I guess? It would be nice to declare this only once and (I guess?) for the declaration to include a version pin.

itamarst (Migrated from github.com) reviewed 2023-04-25 12:04:08 +00:00

tox.ini

itamarst (Migrated from github.com) commented

2023-04-25 12:04:08 +00:00

I have a follow-up PR that I'll open once this one is merged which adds pinning.

itamarst (Migrated from github.com) reviewed 2023-04-25 12:04:58 +00:00

tox.ini

itamarst (Migrated from github.com) commented

2023-04-25 12:04:58 +00:00

Actually, no, I didn't. So I'll make them match.

itamarst (Migrated from github.com) reviewed 2023-04-25 12:06:02 +00:00

tox.ini

itamarst (Migrated from github.com) commented

2023-04-25 12:06:02 +00:00

The reason for duplication is faster CI; it allows not installing everything on each CI run, just ruff. Not quite sure how to fix that... maybe a .ruff.version file?

The reason for duplication is faster CI; it allows not installing everything on each CI run, just ruff. Not quite sure how to fix that... maybe a `.ruff.version` file?