Draft: Add a pylint test to check the code quality

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 19, 2023, 11:30

Add a pylint test to check the code quality. Many checks are currently disabled and should be fixed one-by-one.

TODO: reconsider using ruff or other precommit check.

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 19, 2023, 11:30

requested review from @bentvelsen

[SGeeversAtVortech]

In GitLab by @bentvelsen on Sep 20, 2023, 09:56

approved this merge request

[SGeeversAtVortech]

In GitLab by @vreeken on Sep 21, 2023, 22:30

Commented on .gitlab-ci.yml line 12

Why a separate stage? Won't that make running the tests slower?

[SGeeversAtVortech]

In GitLab by @vreeken on Sep 21, 2023, 22:30

Any particular reason for choosing pylint over ruff?

Any particular reason to not move style checks to pre-commit (yet)?

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 22, 2023, 09:02

Commented on .gitlab-ci.yml line 12

The style check is relatively short, so I don't expect it will make a lot of difference. It would be nice to do the style check precommit (weren't you working on something like this?). Than we just have the code quality check here.

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 22, 2023, 09:03

No particular reason. It seems pylint is a lot more extensive.

[SGeeversAtVortech]

In GitLab by @vreeken on Sep 22, 2023, 09:58

Yep, it's apparently also a lot slower, but I haven't played around with it myself. Mostly matters when running pre-commit obviously, because then we might get annoyed with slowness of a linter ;)

[SGeeversAtVortech]

In GitLab by @vreeken on Sep 22, 2023, 09:59

Commented on .gitlab-ci.yml line 12

@ruudk_rhdhv made that comment a while back about both of them. And purely by coincidence (it wasn't me but Kent that opened the MR a few days back), we're working on implementing pre-commit with black/flake8 and ruff/pylint as well in Pymoca https://github.com/pymoca/pymoca/pull/296

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 22, 2023, 10:30

There are several checks in pylint like too-many-statements, too-many-branches, duplicate-code that I would like to enable in the future that don't seem to be supported by ruff at the moment. I agree that pylint is too slow for a pre-commit check.

[SGeeversAtVortech]

In GitLab by @ruudk_rhdhv on Sep 22, 2023, 10:40

@SGeeversAtVortech are you sure it's not supported by ruff (see https://docs.astral.sh/ruff/rules/) or by another pre-commit hook (see https://github.com/pre-commit/pre-commit-hooks)?

We use this for example for our .pre-commit-config.yaml:

repos:
-   repo: https://github.com/psf/black
    rev: 23.1.0
    hooks:
    -   id: black
-   repo: https://github.com/pre-commit/pre-commit-hooks
    rev: v4.4.0
    hooks:
    -   id: check-toml
    -   id: check-yaml
    -   id: trailing-whitespace
-   repo: https://github.com/charliermarsh/ruff-pre-commit
    rev: 'v0.0.272'
    hooks:
    -   id: ruff
        args: [--fix, --show-fixes]

[SGeeversAtVortech]

In GitLab by @ruudk_rhdhv on Sep 22, 2023, 10:41

FYI: you can use pre-commit run --all-files --show-diff-on-failure in your pipeline to directly fail the linting step if your pre-commit doesn't pass.

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 22, 2023, 11:05

@ruudk_rhdhv You're right, the too-many checks are included in ruff now. I was looking at an older discussion. Then we might reconsider and use ruff instead.

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Sep 22, 2023, 11:08

marked this merge request as draft

[SGeeversAtVortech]

In GitLab by @SGeeversAtVortech on Nov 14, 2023, 16:14

We now use precommit checks with black !464 and ruff !466 instead.