Request for proposals/comments: Contributing guidelines - linting

[nuclearcat]

Since i am working on contributing guidelines, probably it will be good idea to unify linting across the projects to follow some standards, because right now it is very hard to explain for contributors, why rules vary across the projects.
We can start on python:

AI-assisted summary:

• For Python-only projects, the strictness (from strongest to weakest in this summary) is:
  - kernelci-core: pycodestyle + pylint + mypy in CI.
  - dashboard backend: flake8 + black --check (max-complexity = 10, max-line-length = 110).
  - kci-dev: black + isort + tox checks.
  - kernelci-api: pycodestyle + pylint (no black/isort/mypy in the listed config).
  - kernelci-pipeline: pycodestyle only.
  - kcidb-ng: no Python lint jobs in listed workflows. (but i think after moving ingester and logspec to dashboard we wont have python there.

What i propose to take for reference kci-dev + dashboard derived rules:
black --check + isort --check --diff
then flake8:

      - max-line-length = 110
      - max-complexity = 10

with reasonable ignore list.
Also for sure we use appropriate way to run embedded tests.

Probably also tuxmake and other projects we will need to convert too.

[bhcopeland]

Makes sense to me that we adhere to that. I would suggest using ruff though, it puts all three tools into one. We should also define an ignore acceptable list, i.e. E203, W503, etc. Without a pinned list, each project will pick different ignores and contributors are back to guessing.

max-complexity=10 needs an escape hatch policy. my worry here looking at tuxmake is init classes exceed this, and we use # noqa: C901 here. I deem this as acceptable, but it should be in the policy.

I see you added mypy, that's fine, but should mypy be part of the standard?

For tuxmake, we default to -88 lines. Should we be explicit that black --line-length 110 is the rule?

[nuclearcat]

mypy is used in another repos, i think i will remove it.
So my proposal: black, isort, flake8. I guess ruff can replace all of that as you said (but somehow it is a bit different than black)
Reasonable exceptions is ok, especially if it is old code and rewriting such parts are too risky/complex.

[victor-accarini]

I agree, it would be nice. I prefer keeping the tooling to a minimal, so if we could check how much using ruff would disrupt the current codebase would be nice. @gustavobtflores can you do a quick check on how much changing to ruff would impact the current codebase? (test only the dashboard - is more like a feasibility check)

[gustavobtflores]

We would need to reformat some files, even using the same rules, because there are differences in how Ruff formats files compared to Black.
In my tests, Ruff reformatted 58 files, but most changes were only 1~2 lines. There were also some duplicate keys on dicts that Black apparently allowed.

[nuclearcat]

@aliceinwire, I'd love to get your thoughts on this. Once we reach a consensus, my plan is to first get all mergeable PRs merged to avoid conflicts. After that, I will align all the repositories to use these unified CI/CD checks. The goal here is to significantly reduce the maintenance burden on individual repo maintainers, shift that workload over to the sysadmin team, and make cross-repository contributions much smoother. Let me know what you think.

[tales-aparecida]

I recommend starting with ruff + type checker + pre-commit hooks

For pre-commit hooks I personally use the framework from pre-commit.com, and I like how easy it is to add more checks to it, but we could create our own (containerized) solution, if we don't want to adopt it.

Type checkers cover a different set of problems from basic linters. It could be mypy, or even Pyright, but the ruff people have a Rust based https://docs.astral.sh/ty/ which is also fast and might be good, I have never tried it. I found that there's also another Rust checker called pyrefly which apparently caches more problems than ty, but that is not necessarily a good thing.

Ruff is fast and already covers a great part of the most problematic checks you'd get from other linters. With regard to rules, I'm in the extreme opposite 😅 , I suggest using as many defaults as possible, enabling all rules and ignoring only those we actively disagree with, and leave a justification around them. I realize it would take a huge effort to get projects up to standard, so we'd need live with warnings forever. So my second suggestion is to allow each project to handle their own list of ignored rules, and start with whatever list is necessary to pass ruff linting without warnings. Then create a "GoodFirstIssue" in each of them to enable more rules and fix their code smells.

[tales-aparecida]

About line length, CKI uses 100, with the exception for 120 in Django projects.

If we opt to use ruff, we would be able to set a soft and a hard limit https://docs.astral.sh/ruff/settings/#lint_pycodestyle_max-line-length

Soft limit is good to avoid incentivizing people to aim for the max length. For example, we could set a policy of 80/110, which would allow devs to rely on the auto-formatter to wrap lines over 80 chars, and fail linting only if they write something, like a long_named_variable for a long string with over 110 chars.

[tales-aparecida]

About complexity and escape hatch for tuxmake. Ruff allows ignore rules for globs/paths, so we could handle that.

[tales-aparecida]

For running tests, CKI has been using tox+pytest, but poetry and uv (from the ruff people) is more modern and probably better. Python native unittest is usually enough, but pytest has some QoL filters that help me a lot. :)