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

Use Ruff as a linter #1004

Merged
merged 5 commits into from
Feb 21, 2023
Merged

Conversation

NickCrews
Copy link
Contributor

Here is an example PR, it probably will need to get rebased once #1003 is merged.

No pressure if this isn't what you;re into, but I LOVE
this sort of auto-formatter/auto-fixing tool, so I thought
I'd just write a PR.

I really wanted this for isort, because I really like isort.
It reduces the likelihood of merge conflicts
and just makes the imports more tidy.

But ruff has other advantages too:

  • it does everything that flake8 does
  • bugbear and pyupgrade support for more linting
  • its MUCH faster, so its reasonable to have as a pre-commit hook.
  • allows us to delete the .flake8 config file

It is MUCH faster (making it reasonable
to use as a pre-commit hook),
and supports isort and bugbear
so we can catch more problems,
and it supports autofixing.

I'll enable bugbear and isort in a followup, but you can see here
how much of a drop it replacement it is.
This actually caught a bug in a test
where we weren't actually asserting anything.
@RobinL
Copy link
Member

RobinL commented Feb 2, 2023

Hiya. Thanks for this - just checked with the team and we're all happy to migrate to Ruff. We have quite a big incoming PR here so I'd probably be inclined to do it after we merge that, if that's ok

@NickCrews
Copy link
Contributor Author

Definitely, get that big hefty one merged :) This will be pretty easy to rebase: just cherry pick the pyproject.toml and github actions workflow, then run ruff and it will autofix pretty much everything else.

@ThomasHepworth ThomasHepworth changed the base branch from master to ruff February 21, 2023 14:33
@ThomasHepworth ThomasHepworth merged commit 311356b into moj-analytical-services:ruff Feb 21, 2023
@NickCrews
Copy link
Contributor Author

@ThomasHepworth I don't think you wanted to merge that, it really needed to be rebased on main first. The CI tests above all are unhappy

@ThomasHepworth
Copy link
Contributor

@NickCrews don't worry, I've just migrated it to my fork so I can test get the ghub actions sorted.

It's just on a separate branch called ruff for now.

@NickCrews NickCrews deleted the ruff branch February 14, 2024 21:15
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