Skip to content

Bump ruff and remove sort-all #4075

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.

Sign up for GitHub

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

Jump to bottom
Merged
Bibo-Joshi merged 3 commits into from
Jan 24, 2024
Merged

Bump ruff and remove sort-all #4075

Bibo-Joshi merged 3 commits into from
Jan 24, 2024

Conversation

@harshil21
Copy link
Member

@harshil21 harshil21 commented Jan 21, 2024 •
edited
Loading

Removes sort-all since ruff now supports sorting __all__. Also adds 2 more ruff rules:

Also forces pre-commit to use python 3.12, to ensure consistency between different hooks and between locally run pre-commit and CI pre-commit

@harshil21 harshil21 added the 🛠 code-quality change type: code-quality label Jan 21, 2024
github-actions[bot]
github-actions bot reviewed Jan 21, 2024
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hey! Looks like you edited the (optional) requirements or the pre-commit hooks. I'm just a friendly reminder to keep the additional dependencies for the hooks in sync with the requirements :)

@github-actions github-actions bot removed the 🛠 code-quality change type: code-quality label Jan 21, 2024
@harshil21 harshil21 added the 🛠 code-quality change type: code-quality label Jan 21, 2024
@harshil21
Copy link
Member Author

harshil21 commented Jan 21, 2024

huh, my pre-commit did not report those errors (differences in python version?).. also it seems like SIM401 was silently being ignored all this time? It's not a preview rule so not sure how ruff didn't catch that before..

Bibo-Joshi
Bibo-Joshi reviewed Jan 21, 2024
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

different python version could certainly be. ruff always wants me to use typing.Self when running on py3.11 …

@harshil21
Copy link
Member Author

harshil21 commented Jan 21, 2024

Interesting thing I found about pre-commit - new contributors will not need to run pre-commit install whenever they clone our repo, if we do this. It will automatically install the hooks during cloning and run them on git commit. I can try doing this on another PR.

@harshil21
Try to force pre-commit to use py3.12
35c2d32 
Also fix satisfy ruff errors
@harshil21
Copy link
Member Author

harshil21 commented Jan 22, 2024

Interesting thing I found about pre-commit - new contributors will not need to run pre-commit install whenever they clone our repo, if we do this. It will automatically install the hooks during cloning and run them on git commit. I can try doing this on another PR.

well I misunderstood what that option is for. It's only meant for local setups where you want your git to automatically install the .pre-commit-config.yaml from repositories to .git/hooks. We can't instruct others' git to do that (which makes sense since the INSTALL_PYTHON variable in .git/hooks/pre-commit differs according to where python is installed in your system).

Bibo-Joshi
Bibo-Joshi reviewed Jan 22, 2024
View reviewed changes
@harshil21
Remove pre-commit version specifier
0dfca0d 
And tell mypy to be 3.8 compliant
Bibo-Joshi
Bibo-Joshi approved these changes Jan 24, 2024
View reviewed changes
Copy link
Member

@Bibo-Joshi Bibo-Joshi left a comment

Choose a reason for hiding this comment

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

LGTM :) Is there anything left from your side or should I merge?

@harshil21
Copy link
Member Author

harshil21 commented Jan 24, 2024

@Bibo-Joshi all good from my side

@Bibo-Joshi Bibo-Joshi merged commit 2d63c57 into master Jan 24, 2024
@Bibo-Joshi Bibo-Joshi deleted the rm-sort-all-ext branch January 24, 2024 19:53
@github-actions github-actions bot locked and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@Bibo-Joshi Bibo-Joshi Bibo-Joshi approved these changes

Assignees

No one assigned

Labels

🛠 code-quality change type: code-quality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@harshil21 @Bibo-Joshi