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. :)

[MarceloRobert]

@tales-aparecida pre-commit sounds pretty interesting, specially if it works for mono-repos like the dashboard. As for the tools, I think that more speed is very welcome here, so I'd prefer ruff + ty. If the OpenAI acquisition is a problem, then I'd say to use black + flake8 + mypy, which are the usual trusted tools.

I don't mind the line-length rule that much, 110 feels ok in the dashboard but it's fine to change it; I do think that we should have this centralized in some way though. I also agree that we should be as strict as possible with the rules, and only deselect the ones we actively disagree.

About rule exceptions, I'd say it's good to have a definition of what can enter the exceptions, something like "starting functions of a module, command or subcommand or old functions that are complex to refactor". Outside of that I have no more suggestions.

cc @bhcopeland @nuclearcat @gustavobtflores @victor-accarini

[tales-aparecida]

wrt to the OpenAI acquisition, my current position is to watch the projects carefully and jump into a fork in case something feels off

[nuclearcat]

So my current summary:

Linting and Formatting

Python: ruff (linting + import sorting) in CI/CD and in pre-commit hooks.
Some changes from default, to relax rules:

• project-level ignore list with rationale
• line length soft 80/hard 110
• Allow scoped esceptions (legacy, high complexity entrypoints)

Pre-commit

All repositories SHOULD include a .pre-commit-config.yaml
Hooks MUST run Ruff checks and formatting
CI MUST enforce the same checks

Type Checking (Optional)

Rather i prefer to not, but if projects dont have much contributors and maintainers prefer that, type checking MAY be enabled per project

• mypy or pyright

Type checking MUST NOT block adoption of the standard linting baseline.