SeedSigner / seedsigner

Use an air-gapped Raspberry Pi Zero to sign for Bitcoin transactions! (and do other cool stuff)
MIT License
699 stars 161 forks source link

Enable code linting (static code analysis) via pre-commit + ruff and fix findings #584

Open dbast opened 2 months ago

dbast commented 2 months ago

Description

linting as in static code analysis to improve the overall code quality (this is not about code style!)

pre-commit: The pre-commit framework became pretty widely used in recent years, especially in the Python world, where it originates from. It enables combining multiple hooks and has its own dependency management. There are many hooks for different routine checks like linting (and also code generation / formatting), see also https://pre-commit.com/hooks.html. The hooks are configured in a file called .pre-commit-config.yaml and so don't pollute the requirements*.txt files. pre-commit can be added as git pre-commit hook via running pre-commit install or just be executed on cli (or IDE on every file change) via pre-commit run --all. Added this also as comments to the config file. With clever selections of hooks a runtime of <5 seconds can be achieved for a good developer experience, when running on every change. The config contains some standard hooks and ruff as Python code linter.

ruff: https://github.com/astral-sh/ruff is proposed as Python linter as it is super fast (due to being implemented in Rust) and it implements lots of lints/tests from flake8/bandit/pyupgrade/pylint and more and more big projects adopted it already. The elegant thing with ruff is that it proposes fixes for found issues and the unified configuration via pyproject.toml, no matter what rule category gets activated or has to be partially ignored.

This PR introduces pre-commit as concept and ruff with only the test category "F" (=pyflakes) and "T20" (=flake8-print) enabled to keep the amount of fixes reviewable. An added GH workflow runs it on every PR to check that there are no findings.

The category "F" finds lots of things, that are fixed by this PR, like duplicated imports, un-used return-values, un-reachable code, un-used variables, hard to follow star imports etc.

Further PRs can add more hooks and/or enable more ruff rule categories, if sensible.

This pull request is categorized as a:

Checklist

If you modified or added functionality/workflow, did you add new unit tests?

I have tested this PR on the following platforms/os:

Note: Keep your changes limited in scope; if you uncover other issues or improvements along the way, ideally submit those as a separate PR. The more complicated the PR the harder to review, test, and merge.