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

Replace black, isort and flake8 with ruff #725

Merged
merged 17 commits into from
Apr 23, 2024
Merged

Replace black, isort and flake8 with ruff #725

merged 17 commits into from
Apr 23, 2024

Conversation

thekaranacharya
Copy link
Contributor

@thekaranacharya thekaranacharya commented Apr 22, 2024

  • Gets rid of black, isort and flake8, and replaces them with ruff
  • Updates the pyproject.toml with ruff settings
  • Updates the makefile: make lint and make autoformat with the correct commands
  • Remove flake8 config file
  • Using the default config from here + existing ignores from flake8 and isort
  • Add ruff linting + formatting as a pre-commit hook
  • Other files in this PR that changed are the transformed files after linting + formatting

@thekaranacharya thekaranacharya marked this pull request as ready for review April 23, 2024 00:21
@thekaranacharya
Copy link
Contributor Author

@CalebCourier @zsimjee I added the pre commit config yaml and installed the hook with pre-commit install - that installed the hook in my local guardrails/.git, which is not synced up. What's the best way to have that pre-commit hook here?

  • Should we add a command inside our make precommit here to run pre-commit install?

.flake8 Show resolved Hide resolved
.flake8 Show resolved Hide resolved
@zsimjee
Copy link
Collaborator

zsimjee commented Apr 23, 2024

@CalebCourier @zsimjee I added the pre commit config yaml and installed the hook with pre-commit install - that installed the hook in my local guardrails/.git, which is not synced up. What's the best way to have that pre-commit hook here?

  • Should we add a command inside our make precommit here to run pre-commit install?

installing precommit should be included in make dev/full/all.

CalebCourier
CalebCourier previously approved these changes Apr 23, 2024
Copy link
Collaborator

@CalebCourier CalebCourier left a comment

Choose a reason for hiding this comment

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

LGTM!

@thekaranacharya
Copy link
Contributor Author

@CalebCourier @zsimjee I added the pre commit config yaml and installed the hook with pre-commit install - that installed the hook in my local guardrails/.git, which is not synced up. What's the best way to have that pre-commit hook here?

  • Should we add a command inside our make precommit here to run pre-commit install?

installing precommit should be included in make dev/full/all.

Added it to make dev - as makes sense to have them when we're adding new code

@zsimjee
Copy link
Collaborator

zsimjee commented Apr 23, 2024

I pulled down the PR and tested the precommit hook, it doens't seem to work. I added a line with really long line, and it passed the hook

@zsimjee
Copy link
Collaborator

zsimjee commented Apr 23, 2024

but it worked on an unused variable!

@thekaranacharya
Copy link
Contributor Author

I pulled down the PR and tested the precommit hook, it doens't seem to work. I added a line with really long line, and it passed the hook

@zsimjee Added the E501 check to test for line too long. Now tested in a fresh environment, added a new long line example in another file, the pre commit hook runs and fails - thereby stopping the commit!

@zsimjee zsimjee merged commit 6cb406b into main Apr 23, 2024
20 checks passed
@zsimjee zsimjee deleted the karan/ruffify branch April 23, 2024 14:11
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