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

Enable+apply isort via ruff/pre-commit #1871

Merged
merged 5 commits into from
Aug 21, 2024
Merged

Enable+apply isort via ruff/pre-commit #1871

merged 5 commits into from
Aug 21, 2024

Conversation

dbast
Copy link
Contributor

@dbast dbast commented Aug 17, 2024

This enables isort via ruff/pre-commit and applies it fully automatically (no manual changes), which results in sorting of the imports (no additions or removals).

While this looks like a cosmetic thing, the intention is different:

  • Sorting in to sections (stdlib, 3rd party libs, own code..): Ensures that any 3rd party / own code does not overwrite imports from stdlib and the sections also help to see what comes from where.
  • Alphabetical ordering within sections: helps to find things within sections, when reading the imports.

This now also runs the black hook after ruff, as reordering imports can result in some work for black.

@dbast dbast marked this pull request as draft August 17, 2024 08:01
@dbast dbast closed this Aug 17, 2024
@dbast dbast reopened this Aug 17, 2024
@dbast dbast marked this pull request as ready for review August 18, 2024 07:44
- repo: https://github.com/psf/black
rev: 24.8.0
hooks:
- id: black
- repo: https://github.com/astral-sh/ruff-pre-commit
Copy link
Collaborator

Choose a reason for hiding this comment

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

wasn't ruff enabled in the previous commit? Do you know why it did not it apply isort previously?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, but the previous PR only enabled the F ruleset, see https://github.com/Consensys/mythril/blob/develop/pyproject.toml#L7 ... to find and remove unused code/imports.

ruff has many more rules that can be activated, see https://docs.astral.sh/ruff/rules/

This PR enables isort by adding the I to the activated rules, see https://github.com/dbast/mythril/blob/isort/pyproject.toml#L7

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, I don't see that in this PR. Is the I in a different PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nvm, found it,
Github compressed some files for me

Copy link
Collaborator

@norhh norhh 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 contribution!

@norhh norhh merged commit 0717111 into Consensys:develop Aug 21, 2024
5 checks passed
@dbast dbast deleted the isort branch August 23, 2024 08:16
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