Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace black, flake8, pyupgrade with ruff for Python linting and formatting #3248

Merged
merged 4 commits into from
Jun 3, 2024

Conversation

flodolo
Copy link
Collaborator

@flodolo flodolo commented May 30, 2024

No description provided.

@flodolo
Copy link
Collaborator Author

flodolo commented May 30, 2024

Thoughts on adding ruff on top of black? Does it need to be called out in contributing.rst? Should we drop black alltogether?

@flodolo flodolo changed the title Add Ruff to CI Use Ruff for Python linting in CI May 30, 2024
@flodolo
Copy link
Collaborator Author

flodolo commented May 31, 2024

Looking at AMO, they completely dropped black for ruff, and use some configuration options
https://github.com/eviljeff/olympia/blob/9c570b5180b02a6e5d9e0d1d1dc550d76b3c249e/pyproject.toml

I guess it should be replacing both flake8 and black at this point 🤔
https://docs.astral.sh/ruff/

@mathjazz
Copy link
Collaborator

Seems like ruff would let us drop black, flake8 and even pyupgrade.

All that while being faster.

Sounds like a win-win to me.

@flodolo
Copy link
Collaborator Author

flodolo commented May 31, 2024

Oh, I missed pyupgrade.

Let me play with it a bit more, especially with the configuration part, and see how far I can go.

@flodolo flodolo changed the title Use Ruff for Python linting in CI Replace black, flake8, pyupgrade with ruff for Python linting and formatting May 31, 2024
@flodolo
Copy link
Collaborator Author

flodolo commented May 31, 2024

There are quite a few changes running ruff format to replace black:

Not completely sure what we want to add/enable in the TOML file.

EDIT: I suppose that we could exclude */migrations/* from listing/formatting, but is there a reason for doing it?

@mathjazz
Copy link
Collaborator

mathjazz commented May 31, 2024

@flodolo
Copy link
Collaborator Author

flodolo commented May 31, 2024

Space after the class definition: formatter removes blank line after class definition astral-sh/ruff#8566

Disable that?

https://docs.astral.sh/ruff/rules/one-blank-line-after-class/

I don't see a way to disable it in the format part (while it would be possible in the linting), very much like black wasn't configurable.

@flodolo
Copy link
Collaborator Author

flodolo commented May 31, 2024

See also astral-sh/ruff#9745

Copy link
Collaborator

@mathjazz mathjazz left a comment

Choose a reason for hiding this comment

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

I don't see a way to disable it in the format part (while it would be possible in the linting), very much like black wasn't configurable.

Oh, I see...

We talked about this PR at the call today and we all agreed we should go ahead.

@mathjazz
Copy link
Collaborator

mathjazz commented Jun 3, 2024

Also, nobody expressed strong opinions the new line after block open and/or about excluding migration files, so let's leave that as it is for now.

Eemeli suggested we also look into https://pycqa.github.io/isort/.

@mathjazz mathjazz merged commit d650739 into mozilla:main Jun 3, 2024
2 checks passed
@flodolo flodolo deleted the ruff branch June 4, 2024 07:56
@flodolo
Copy link
Collaborator Author

flodolo commented Jun 4, 2024

Eemeli suggested we also look into https://pycqa.github.io/isort/.

In case, looks like it can be fine-tuned for Django (see AMO file)

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.

None yet

2 participants