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 ruff linter and formatter to pre-commit #856

Merged
merged 4 commits into from
Jan 13, 2025

Conversation

vgrozdanic
Copy link
Contributor

@vgrozdanic vgrozdanic commented Jan 12, 2025

In the past 2 PRs that i have opened, i have used linter locally to double check the changes and that i haven't forgotten something. I thing all of the devs (of this library) would benefit from having the linter as a part of pre-commit hooks to quickly get the feedback about potential problems in code :)

I have no hard opinions about this specific linter or formatter, but ruff is fast, and it is very easy to setup. I have been using ruff in a few of my personal projects and i really like it, so i have opened a PR to add it to this repo too.

In addition to adding the new linter and formatter, I have also updated extras_require in setup.py since it contained unused packages, and it didn't list all of the packages that are being actively used.

Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Can we replace black and isort with the ruff formatter?

@vgrozdanic vgrozdanic force-pushed the vgrozdanic/add-ruff-linter-to-pre-commit branch from e05f7f0 to da302bf Compare January 13, 2025 18:06
Copy link
Contributor Author

Choose a reason for hiding this comment

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

After adding ruff linter/formatter to pre-commit, this was auto formatted. I can separate this change into another PR before merging this, but since this is only one change, i have bundled it with this PR

hooks:
- id: ruff
types_or: [ python, pyi ]
args: [--select, I, --fix,]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ruff has imports sorting as a part of linter, not formatter, so we need --select I here to force imports sorting

@vgrozdanic vgrozdanic marked this pull request as ready for review January 13, 2025 18:11
@vgrozdanic vgrozdanic requested review from a team and Andrew-Chen-Wang January 13, 2025 18:11
@vgrozdanic vgrozdanic changed the title Add ruff linter to pre-commit Add ruff linter and formatter to pre-commit Jan 13, 2025
Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

lgtm thanks!

@vgrozdanic vgrozdanic merged commit d4d0aaf into master Jan 13, 2025
21 checks passed
@vgrozdanic vgrozdanic deleted the vgrozdanic/add-ruff-linter-to-pre-commit branch January 13, 2025 20: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.

2 participants