Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
wants to merge 4 commits into
base: dev
Choose a base branch
from
Open

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

wants to merge 4 commits into from

Commits on Jul 29, 2024

  1. Enable code linting via pre-commit 

    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 allows
combine multiple hooks and has its own dependency management. There are
many hooks for different routine checks like linting and code 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 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 the unified
configuration via pyproject.toml, no matter what rule set 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. One exception are the type
annotations in the controller.py, which are non-standard: Decided to add
`# noqa` so that ruff ignores those and resolving those can be tried in a
followup PR.

Further PRs can add more hooks and/or enable more ruff rule categories, if
sensible.
    @dbast
    dbast committed Jul 29, 2024
    Configuration menu
    Copy the full SHA
    5b79af1 View commit details
    Browse the repository at this point in the history

  2. Auto ruff fixes

    @dbast
    dbast committed Jul 29, 2024
    Configuration menu
    Copy the full SHA
    06dd8ea View commit details
    Browse the repository at this point in the history

  3. Manual fixes

    @dbast
    dbast committed Jul 29, 2024
    Configuration menu
    Copy the full SHA
    3166b6c View commit details
    Browse the repository at this point in the history

  4. More fixes for screen short generator

    @dbast
    dbast committed Jul 29, 2024
    Configuration menu
    Copy the full SHA
    241e972 View commit details
    Browse the repository at this point in the history