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

Add some ruff autofixes to CI #10458

Merged
merged 4 commits into from
Jul 20, 2023
Merged

Add some ruff autofixes to CI #10458

merged 4 commits into from
Jul 20, 2023

Conversation

AlexWaygood
Copy link
Member

The aim of this PR is to make it easier for new contributors to typeshed to comply with our -- at times fairly stringent! -- style guide. For large PRs (e.g. #10446, or #10355), this can be quite difficult. Ruff has a number of autofixes that we can use to help out with this task, and it's extremely fast.

Unlike most projects, we're not likely to ever switch from flake8 to ruff -- even if ruff implemented the full set of flake8-pyi rules and had exactly the same behaviour as flake8-pyi for all of them, it's useful for us to be able to add new rules to our own linter whenver we want to. However, there's nothing stopping us from having ruff as part of our CI as well as flake8.

Since (in my opinion) we're only interested in the autofixes here, I've only enabled a few rules that have relatively safe autofixes relevant to stub files.

Since ruff includes isort as one of its autofixes, having isort as a test dependency is now redundant, so I've removed it in favour of ruff's I001 rule.

@github-actions

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented Jul 19, 2023

I'm not a fan of replacing a well-tested, easily usable tool like isort with a new tool that's still under heavy development, like ruff. (And while I'm not a fan of flake8, I'm also not a fan of implementing a linter in a different language for marginal speed wins.)

@srittau
Copy link
Collaborator

srittau commented Jul 19, 2023

And just to clarify: I'm not opposed to using ruff auto-fixes, except for those handled by isort. But I would like to keep isort, at least for now.

@AlexWaygood
Copy link
Member Author

I have some quibbles with your characterisations of ruff vs isort, but I also don't feel strongly at all about replacing isort with ruff (I just thought it would be nice to reduce the number of tools we rely on in CI). So I'll revert replacing isort with ruff, and concentrate on just adding some new autofixes to CI.

@AlexWaygood AlexWaygood requested a review from srittau July 19, 2023 12:19
@github-actions

This comment has been minimized.

@srittau
Copy link
Collaborator

srittau commented Jul 19, 2023

One of the advantages of using isort is that tooling already works, for example, VSCode has isort support integrated. And ruff is quite early in its development with a <1 version number, so I wouldn't expect it to be stable in the fixes it outputs. Maybe in the future it makes sense to replace isort, because in general I support the idea to use less tools.

@AlexWaygood
Copy link
Member Author

AlexWaygood commented Jul 19, 2023

One of the advantages of using isort is that tooling already works, for example, VSCode has isort support integrated. And ruff is quite early in its development with a <1 version number, so I wouldn't expect it to be stable in the fixes it outputs. Maybe in the future it makes sense to replace isort, because in general I support the idea to use less tools.

Sure. After my last push, this PR no longer proposes replacing isort with ruff, just adding a few new ruff autofixes on top of isort.

Nonetheless, to flesh out my quibbles with your earlier characterisation for you: my impression is that ruff's import-sorting is reasonably stable at this point, and that they're very quick to fix bugs when they're filed against the project. I also like that ruff has very regular releases, whereas isort releases are very irregular (and I'd wager they're likely to become increasingly irregular, since the creator of isort has said that he himself is a fan of ruff these days). And isort definitely isn't bug-free (which I know isn't a claim you ever made, but I think speaks to the value of using a tool with regular releases). Personally I find ruff to be just as easily usable as isort -- but then again, I'm not a heavy user of VSCode, so I can't really speak to that aspect.

FWIW, I was also pretty sceptical initially about the idea of a Python development tool written in a different language, just for a bit of a speedup. But I think my scepticism has been proven wrong there, at least for now -- they seem to be getting a lot of PRs from the community, and it seems to be a pretty thriving project. We'll see how it plays out in the long term, of course.

@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@srittau srittau merged commit 79e092e into python:main Jul 20, 2023
@AlexWaygood AlexWaygood deleted the ruff branch July 20, 2023 12:43
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