Add ruff docs and configuration #311

					
				@ -35,3 +35,3 @@

				    steps:

				      - name: Install Python, nodejs, git, and build dependencies

				        run: dnf -y install nodejs git python3.11 python3.11-pip python3.11-setuptools-wheel python3.11-setuptools krb5-devel python3.11-devel gcc

				        run: dnf -y install nodejs git python3.11 python3.11-pip python3.11-setuptools-wheel python3.11-setuptools krb5-devel python3.11-devel gcc sqlite-devel

why does this pr need sqlite-devel?

This fixes failing test suite to demonstrate that this PR is otherwise not breaking any tests.

adamwill marked this conversation as resolved

adamwill reviewed

2026-06-02 14:31:52 +00:00

docs/source/development.rst Outdated

					
				@ -197,0 +202,4 @@

				isort, Black, pycodestyle, Pyflakes, and more) with a single, unified tool that

				runs orders of magnitude faster. Ruff supports over 800 lint rules, can

				automatically fix many of them, and also provides an opinionated code formatter

				compatible with Black.

We don't need this blurb. A simple "This project uses Ruff <https://docs.astral.sh/ruff/>_ for linting and formatting." would suffice.

We don't need this blurb. A simple "This project uses `Ruff <https://docs.astral.sh/ruff/>`_ for linting and formatting." would suffice.

👍 1

jgroman marked this conversation as resolved

adamwill reviewed

2026-06-02 14:32:29 +00:00

docs/source/development.rst Outdated

					
				@ -197,0 +215,4 @@

				  sudo dnf install ruff

				Project configuration

this section is way too long. Nobody wants to read a tedious LLM-generated readout of the config file. Reduce it to bits humans might be interested in, or, if the comments in the file itself are sufficient, remove this section entirely. Also no human would paste in a chunk of the config file that is commented to explain what it does, then explain the same things below in more tedious detail.

this section is way too long. Nobody wants to read a tedious LLM-generated readout of the config file. Reduce it to bits humans might be interested in, or, if the comments in the file itself are sufficient, remove this section entirely. Also no human would paste in a chunk of the config file that is commented to explain what it does, then *explain the same things below* in more tedious detail.

Well, I for one was not very familiar with ruff linter settings and welcomed a little refresher. But you are right about the duplicity.

That's the seductive nature of AI - whatever it generates may well be kinda interesting at the time. But we have to take a step back and say "yes, but does it belong here?" or else we'll wind up with doc files the size of fantasy novels.

Look at how many lines this one feature PR adds to the doc, now imagine if every feature PR did that, what would the doc look like? It's not sustainable.

That's the seductive nature of AI - whatever it generates may well be kinda interesting *at the time*. But we have to take a step back and say "yes, but does it belong here?" or else we'll wind up with doc files the size of fantasy novels. Look at how many lines this one feature PR adds to the doc, now imagine if *every* feature PR did that, what would the doc look like? It's not sustainable.

I think we should just say that we're using Ruff, and that it's configured in pyproject.toml. All config options can be documented inside that file (and most already are). This doc section feels like triplicating that info.

Instead, let's include a link to Ruff config documentation inside pyproject.toml, so that people can find out what a particular option means in detail.

I think we should just say that we're using Ruff, and that it's configured in `pyproject.toml`. All config options can be documented inside that file (and most already are). This doc section feels like triplicating that info. Instead, let's include a link to Ruff config documentation inside `pyproject.toml`, so that people can find out what a particular option means in detail.

jgroman marked this conversation as resolved

adamwill reviewed

2026-06-02 14:34:46 +00:00

docs/source/development.rst Outdated

					
				@ -197,0 +290,4 @@

				Editor integration

				------------------

				Ruff provides first-class editor extensions, giving you real-time diagnostics,

Again, this is advertising copy. We don't need it.

👍 1

jgroman marked this conversation as resolved

adamwill reviewed

2026-06-02 14:38:10 +00:00

docs/source/development.rst Outdated

					
				@ -197,0 +293,4 @@

				Ruff provides first-class editor extensions, giving you real-time diagnostics,

				quick-fix actions, and format-on-save support.

				**VS Code**

I'm kinda on the fence about the laundry list of IDEs here and in the blockerbugs devcontainers PR. On the one hand, sure, I guess it's useful? On the other hand, what is the end state of this approach, every software project in the world duplicates "how to set up your IDE" instructions for every IDE under the sun? I sort of feel like a generic "Ruff extensions are available for many IDEs, you should be able to find instructions with your favorite search engine or chatbot" or something would suffice?

This was sort of requested by Kamil, at least this is how I read it, just replaced git pre-commit hook by format-on-save:

Introducing ruff, e.g. in documentation. Perhaps include a hint on how to set up a git pre-commit hook or something (even though Adam said he commits first and then reformats, so perhaps he has a better approach).

But we can just put some pointers in there instead for sure.

This was sort of requested by Kamil, at least this is how I read it, just replaced git pre-commit hook by format-on-save: > Introducing ruff, e.g. in documentation. Perhaps include a hint on how to set up a git pre-commit hook or something (even though Adam said he commits first and then reformats, so perhaps he has a better approach). But we can just put some pointers in there instead for sure.

yeah, I guess I'm saying make it a bit shorter and more generic rather than step-by-step instructions for every IDE in the world :D

And yes, I tend to write the code and make it work, commit it (locally, no push), then run the formatter. That way, I can just run git diff and see exactly what the formatter did, and easily revert its changes if I don't like them and want to adjust something manually and run the formatter again, or whatever.

yeah, I guess I'm saying make it a bit shorter and more generic rather than step-by-step instructions for every IDE in the world :D And yes, I tend to write the code and make it work, commit it (locally, no push), *then* run the formatter. That way, I can just run `git diff` and see exactly what the formatter did, and easily revert its changes if I don't like them and want to adjust something manually and run the formatter again, or whatever.

Adam, Jaroslav, do you use any pre-commit/pre-push hook? Or rely just on manual execution, and/or IDE integration?

I think this section is fine, but I'd move it to development_tasks.rst instead. There are more guides like these, it fits there.

If we can make it shorter, e.g. by linking somewhere where these instructions already exist, we certainly can do that. E.g. "Configuring VS Code" at https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff seems to contain similar information. But it's harder to follow (more information) and the section can't be linked directly, so I'm completely sure what's better.

Adam, Jaroslav, do you use any pre-commit/pre-push hook? Or rely just on manual execution, and/or IDE integration? I think this section is fine, but I'd move it to `development_tasks.rst` instead. There are more guides like these, it fits there. If we can make it shorter, e.g. by linking somewhere where these instructions already exist, we certainly can do that. E.g. "Configuring VS Code" at https://marketplace.visualstudio.com/items?itemName=charliermarsh.ruff seems to contain similar information. But it's harder to follow (more information) and the section can't be linked directly, so I'm completely sure what's better.

I am usually happy with whatever formatter outputs and use format-on-save IDE feature. However this is only useful for new or already preformatted codebases. When working on existing code base like this I either disable formatter completely or just format new code parts manually (VSCode supports formatting just the selected lines).
It is up for a debate whether to enforce formatting/linting strictly using Forge pre-commit check or just make a quick check using CI task. As for me CI task is good enough.

I am usually happy with whatever formatter outputs and use format-on-save IDE feature. However this is only useful for new or already preformatted codebases. When working on existing code base like this I either disable formatter completely or just format new code parts manually (VSCode supports formatting just the selected lines). It is up for a debate whether to enforce formatting/linting strictly using Forge pre-commit check or just make a quick check using CI task. As for me CI task is good enough.

Ok, I think I'd also avoid hooks at this moment. It can always be added later.

jgroman marked this conversation as resolved

adamwill force-pushed feature/306-1-introducing-ruff from 78f77f2d2e

Run tests and linters / test (pull_request) Successful in 2m56s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

to e85b5fbc33

Run tests and linters / test (pull_request) Successful in 3m2s

Details

Run tests and linters / lint (pull_request) Failing after 3s

Details

2026-06-02 22:44:33 +00:00

Compare

adamwill commented

2026-06-02 22:45:03 +00:00

I extracted the test fix from this PR and merged it, so we aren't starting from a base of broken tests. I've rebased this branch.

kparal requested changes

2026-06-03 12:37:25 +00:00

Dismissed

docs/source/development.rst Outdated

					
				@ -197,0 +209,4 @@

				Install Ruff into your virtualenv::

				  pip install ruff

This is already done in a previous step pip install -r requirements-development.txt. I think we can drop the install section.

This is already done in a previous step `pip install -r requirements-development.txt`. I think we can drop the install section.

jgroman marked this conversation as resolved

docs/source/development.rst Outdated

					
				@ -197,0 +273,4 @@

				Check for lint issues::

				  ruff check .

I think the dot is not needed in any of these examples

jgroman marked this conversation as resolved

docs/source/development.rst Outdated

					
				@ -197,0 +279,4 @@

				  ruff check --fix .

				Format code (equivalent to Black)::

Let's also say e.g. "Make sure to commit your changes first, if you want to inspect the diff".

jgroman marked this conversation as resolved

docs/source/development.rst Outdated

					
				@ -197,0 +285,4 @@

				Check formatting without modifying files::

				  ruff format --check .

I guess ruff format --diff is more useful here? --check seems useful for pre-commit hooks, etc.

I guess `ruff format --diff` is more useful here? `--check` seems useful for pre-commit hooks, etc.

jgroman marked this conversation as resolved

pyproject.toml Outdated

					
				@ -1,6 +1,29 @@

				[tool.black]

				line-length = 100

				[tool.ruff]

We need to exclude some paths, probably using extend-exclude. I think we don't want to cover these:

/alembic/
/versioneer.py
/blockerbugs/_version.py
/docs/source/conf.py

We need to exclude some paths, probably using [extend-exclude](https://docs.astral.sh/ruff/settings/#extend-exclude). I think we don't want to cover these: ``` /alembic/ /versioneer.py /blockerbugs/_version.py /docs/source/conf.py ```

jgroman marked this conversation as resolved

pyproject.toml Outdated

					
				@ -4,0 +18,4 @@

				    "C4",   # flake8-comprehensions (unnecessary list/dict/set wrappers)

				    "SIM",  # flake8-simplify (simplifiable code patterns)

				    "RUF",  # Ruff-specific rules (unused noqa, mutable class defaults, etc.)

				]

We're enabling quite a lot of rules here. I'm a bit concerned about it. The official guide says:

If you're just getting started with Ruff, the default rule set is a great place to start

https://docs.astral.sh/ruff/rules/

The default value is:

Default value: ["E4", "E7", "E9", "F"]

Do you have personal experience with these? I'd rather start with known non-problematic defaults (maybe include isort on top of it), and once we get more experience with it, we can increase the scope and see it if works well for us or causes some issues somewhere. If we start with expert mode from the beginning, I'm afraid it might be overwhelming.

We're enabling quite a lot of rules here. I'm a bit concerned about it. The official guide says: > If you're just getting started with Ruff, the default rule set is a great place to start https://docs.astral.sh/ruff/rules/ The default value is: > Default value: ["E4", "E7", "E9", "F"] Do you have personal experience with these? I'd rather start with known non-problematic defaults (maybe include `isort` on top of it), and once we get more experience with it, we can increase the scope and see it if works well for us or causes some issues somewhere. If we start with expert mode from the beginning, I'm afraid it might be overwhelming.

I think including a rule set that's approx equivalent to flake8 is reasonable. I found the output of this rule set mostly fine, which implies the rule set itself is OK, but I didn't really look at the rules themselves in detail...

I think including a rule set that's approx equivalent to flake8 is reasonable. I found the *output* of this rule set mostly fine, which implies the rule set itself is OK, but I didn't really look at the rules themselves in detail...

Jaroslav already reverted this to default, which is, I believe, a better way to start. But just to clarify - ruff defaults just to a couple specific rules from the E category, and the whole F category. If you enable preview, which is "a collection of unstable features", you receive "an expanded set of default rules that includes rules from the B, UP, and RUF categories, as well as many more". But this went far beyond that, this enabled everything from E, F, W, I, B, UP, C4, SIM and RUF. If you look into those categories, there are lots of rules which are marked as unstable. I don't think this is a reasonable start.

Jaroslav already reverted this to default, which is, I believe, a better way to start. But just to clarify - ruff defaults just to a couple specific rules from the `E` category, and the whole `F` category. If you enable [preview](https://docs.astral.sh/ruff/preview/), which is "_a collection of unstable features_", you receive "_an expanded set of default rules that includes rules from the B, UP, and RUF categories, as well as many more_". But this went far beyond that, this enabled **everything** from `E`, `F`, `W`, `I`, `B`, `UP`, `C4`, `SIM` and `RUF`. If you look into those [categories](https://docs.astral.sh/ruff/rules/), there are lots of rules which are marked as unstable. I don't think this is a reasonable start.

jgroman marked this conversation as resolved

kparal reviewed

2026-06-03 12:42:09 +00:00

pyproject.toml Outdated

					
				@ -4,0 +19,4 @@

				    "SIM",  # flake8-simplify (simplifiable code patterns)

				    "RUF",  # Ruff-specific rules (unused noqa, mutable class defaults, etc.)

				]

				ignore = []

This is the default value, so we can skip this line.

jgroman marked this conversation as resolved

jgroman added 1 commit

2026-06-04 12:57:57 +00:00

Fix review findings: update docs and ruff config

Run tests and linters / test (pull_request) Successful in 2m48s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

c805c568b7

kparal added 1 commit

2026-06-04 17:01:56 +00:00

Tweak documentation and include in ./run lint

Run tests and linters / test (pull_request) Successful in 2m48s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

85a87c98a8

kparal commented

2026-06-04 18:23:07 +00:00

I made some tweaks. I would also adjust ci.yml to simply run ./run lint inside the linting phase (I feel that it would be great if the same exact commands would apply to both local development and CI), but I'm not sure how to override the default python version to 3.11. Thoughts? (On how to do it or whether to delay it to a future PR?)

I made some tweaks. I would also adjust `ci.yml` to simply run `./run lint` inside the linting phase (I feel that it would be great if the same exact commands would apply to both local development and CI), but I'm not sure how to override the default python version to 3.11. Thoughts? (On how to do it or whether to delay it to a future PR?)

jgroman commented

2026-06-05 08:36:18 +00:00

Do we still need to run flake8 during linting? ruff is supposed to replace that and it has flake8 rules enabled even in the default config.

ruff has python version specified in pyproject.toml using target-version = "py311" directive.

Do we still need to run `flake8` during linting? `ruff` is supposed to replace that and it has flake8 rules enabled even in the default config. ruff has python version specified in `pyproject.toml` using `target-version = "py311"` directive.

kparal commented

2026-06-05 11:05:03 +00:00

Do we still need to run flake8 during linting? ruff is supposed to replace that and it has flake8 rules enabled even in the default config.

That's a good question. Here's the output that flake8 currently reports but is not captured by ruff in a default config:

$ flake8 --statistics | grep -v -E '(E4|E7|E9|F)[0-9]{2}'
...
2     E122 continuation line missing indentation or outdented
4     E124 closing bracket does not match visual indentation
5     E127 continuation line over-indented for visual indent
26    E128 continuation line under-indented for visual indent
1     E203 whitespace before ':'
16    E251 unexpected spaces around keyword / parameter equals
5     E261 at least two spaces before inline comment
2     E262 inline comment should start with '# '
2     E265 block comment should start with '# '
1     E275 missing whitespace after keyword
4     E301 expected 1 blank line, found 0
9     E302 expected 2 blank lines, found 1
1     E303 too many blank lines (2)
44    E501 line too long (106 > 100 characters)
1     W391 blank line at end of file

All of these will be resolved with ruff format, so I think we can drop flake8. I'll add a commit.

ruff has python version specified in pyproject.toml using target-version = "py311" directive.

That's good. But we still run mypy from the script as well. And it would also be nice to be able to run ./run test (the same way as in our docs) to execute the test suite. I guess we need to figure out some envvar, if we want to do this.

> Do we still need to run `flake8` during linting? `ruff` is supposed to replace that and it has flake8 rules enabled even in the default config. That's a good question. Here's the output that flake8 currently reports but is not captured by ruff in a default config: ``` $ flake8 --statistics | grep -v -E '(E4|E7|E9|F)[0-9]{2}' ... 2 E122 continuation line missing indentation or outdented 4 E124 closing bracket does not match visual indentation 5 E127 continuation line over-indented for visual indent 26 E128 continuation line under-indented for visual indent 1 E203 whitespace before ':' 16 E251 unexpected spaces around keyword / parameter equals 5 E261 at least two spaces before inline comment 2 E262 inline comment should start with '# ' 2 E265 block comment should start with '# ' 1 E275 missing whitespace after keyword 4 E301 expected 1 blank line, found 0 9 E302 expected 2 blank lines, found 1 1 E303 too many blank lines (2) 44 E501 line too long (106 > 100 characters) 1 W391 blank line at end of file ``` All of these will be resolved with `ruff format`, so I think we can drop flake8. I'll add a commit. > > ruff has python version specified in `pyproject.toml` using `target-version = "py311"` directive. That's good. But we still run `mypy` from the script as well. And it would also be nice to be able to run `./run test` (the same way as in our docs) to execute the test suite. I guess we need to figure out some envvar, if we want to do this.

kparal added 1 commit

2026-06-05 11:14:36 +00:00

drop flake8, add ruff to ci.yml

Run tests and linters / test (pull_request) Successful in 6m52s

Details

Run tests and linters / lint (pull_request) Failing after 3s

Details

eece42241a

jgroman commented

2026-06-05 14:43:45 +00:00

@kparal wrote in #311 (comment):

But we still run mypy from the script as well. And it would also be nice to be able to run ./run test (the same way as in our docs) to execute the test suite. I guess we need to figure out some envvar, if we want to do this.

I think you should be able to specify python version for mypy in pyproject.toml as well:

[tool.mypy]
python_version = "3.11"

Regarding testing I guess you could use pyenv for that or use devcontainers.

@kparal wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/311#issuecomment-817932: > But we still run `mypy` from the script as well. And it would also be nice to be able to run `./run test` (the same way as in our docs) to execute the test suite. I guess we need to figure out some envvar, if we want to do this. I think you should be able to specify python version for `mypy` in `pyproject.toml` as well: ``` [tool.mypy] python_version = "3.11" ``` Regarding testing I guess you could use `pyenv` for that or use devcontainers.

kparal added 1 commit

2026-06-08 12:39:34 +00:00

exclude conf/ in ruff

Run tests and linters / test (pull_request) Failing after 2m29s

Details

Run tests and linters / lint (pull_request) Has been skipped

Details

4a2ea4912b

kparal referenced this pull request

2026-06-08 12:42:18 +00:00

Make ci.yml use devel commands for linting and testing #315

kparal commented

2026-06-08 12:42:44 +00:00

@jgroman wrote in #311 (comment):

Regarding testing I guess you could use pyenv for that or use devcontainers.

Offloaded it to #315

@jgroman wrote in https://forge.fedoraproject.org/quality/blockerbugs/pulls/311#issuecomment-817993: > Regarding testing I guess you could use `pyenv` for that or use devcontainers. Offloaded it to #315

kparal added 1 commit

2026-06-08 12:43:42 +00:00

remember your commas

Run tests and linters / test (pull_request) Successful in 2m45s

Details

Run tests and linters / lint (pull_request) Failing after 2s

Details

a4186d5b44

kparal referenced this pull request

2026-06-08 13:01:49 +00:00

Reformat codebase to satisfy CI linter check #306

kparal approved these changes