Skip to content

Development: introduce Ruff as linter/formatter#3039

Merged
dgtlmoon merged 6 commits into
dgtlmoon:masterfrom
xLinkOut:development/use-ruff
Apr 18, 2025
Merged

Development: introduce Ruff as linter/formatter#3039
dgtlmoon merged 6 commits into
dgtlmoon:masterfrom
xLinkOut:development/use-ruff

Conversation

@xLinkOut
Copy link
Copy Markdown
Contributor

@xLinkOut xLinkOut commented Mar 21, 2025

Hi. For a long time, I've had this repository in my bookmarks, but until recently I had never used it, even though I really liked the idea. Recently, I self-hosted it to track changes of some information on the web, and I found it very useful. In this regard, thanks to you and all the people behind the project!

I decided to contribute, as much as I can, to the project. However, I noticed that no tools are being used for code formatting and linting (black, flake8, ruff...). I believe it's important to have one that at least enforces code formatting, to avoid creating "noise" between PRs and to have a common style for everyone.

This PR introduces Ruff, by Astral, among the dependencies, and its configuration (certainly opinionated by me, it's the one I've been using in all my projects for some time now).

It's easy to notice that a simple ruff check and a ruff format show many changes to the codebase, all naturally style-related, as well as highlighting violations of the style rules specified in the configuration.

Additionally, the PR adds a small ignore line for the configuration file that is created by pyenv, if it is used to use a specific Python version.

PS. If approved, I can take care of applying all the formatting and various fixes that Ruff recommends (both those that can be applied automatically and the "unsafe" ones that require manual application).

Next step: use a separate dev-requirements and/or test-requirements with ruff, pytest and related
Comment thread .ruff.toml Outdated
@@ -0,0 +1,48 @@
# Assume Python 3.11
target-version = "py311"
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we actually test and support 3.10/3.11 and 3.12, so that needs to be taken into account (so the github build workflow, maybe it can be integrated there somehow?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed that the lower supported version was 3.11. I'll change that config to py310, so Ruff know which upgrade suggestions can be safely applied.

@darknetehf
Copy link
Copy Markdown

This is a positive step, I use ruff too but as a pre-commit along with gitleaks and a couple other git hooks.

Is this ruff config supposed to trigger automatically when a commit is made?

@dgtlmoon
Copy link
Copy Markdown
Owner

@xLinkOut
Copy link
Copy Markdown
Contributor Author

Is this ruff config supposed to trigger automatically when a commit is made?

No, not yet. I can configure a pre-commit-hook to automatically apply formatting in this same PR.

hmm dgtlmoon/changedetection.io/actions/runs/13999049165/job/39201447412 no output?

Same for the pipeline.

Also, should I align the ruff configuration with the flake8 params from the workflow?

flake8 . --count --select=E9,F63,F7,F82 --show-source --statistics
flake8 . --count --exit-zero --max-complexity=10 --max-line-length=127 --statistics

@xLinkOut
Copy link
Copy Markdown
Contributor Author

I've added the configuration for the Ruff pre-commit hook. This require to pre-commit install to use it. Also, it's suggested to apply the hook to all file in the repository.

In the test-only Action I've replaced flake8 with ruff, using the .ruff.toml configuration file already in the repo as configuration, without specifying other rules in the second command. By default, Ruff show a list of files that violate a rule, that I found more useful that --statistics flag (they are mutually exclusive unfortunately).

@dgtlmoon
Copy link
Copy Markdown
Owner

This require to pre-commit install

hmm i dont know about doing this, it MUST be automated in the github workflow somehow, i simply dont want to have another command that i will probably forget to run

@xLinkOut
Copy link
Copy Markdown
Contributor Author

I understand your point of view. The command itself can be documented in the readme and/or in a section for developers, on the other hand it only needs to be executed once after cloning the repository.

I firmly believe that maintaining code formatted according to a uniform style brings only long-term benefits, and having the configuration files that determine said style within the repository is positive.

In fact, there are several ways to automate this process: I personally use the Ruff extension for VSCode, set up to format the file I'm working on when I save it. Just like the pre-commit hook, once installed, it acts automatically with each commit. I repeat myself, I believe the benefits far outweigh the overhead of configuring ruff/installing the pre-commit once.

In the GitHub workflow I have already integrated linting, but how would you handle formatting? Should the action format the code applying ruff rules, and then autonomously commit to the repository? I don't even remember if this is possible (due to permissions? idk), otherwise the changes will be lost in the action container

@dgtlmoon
Copy link
Copy Markdown
Owner

hmmm yes i understand, i wonder if pyCharm (my IDE) can support this?

@xLinkOut
Copy link
Copy Markdown
Contributor Author

xLinkOut commented Mar 23, 2025

This is a guide to do the "Format on save" trick.

And yes, Ruff seems to be integrated with PyCharm too.

This page also link to the extension for PyCharm on JetBrains marketplace, that look exactly as the counterpart for VSCode.

@dgtlmoon
Copy link
Copy Markdown
Owner

ok cool,. what is needed here before merge? :)

@xLinkOut
Copy link
Copy Markdown
Contributor Author

It's all ready to merge for me. There is the Ruff config file ready to use (globally or incrementally on files while writing some code) and the pre commit hook to automate this behavior (not mandatory, but nice to have).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants