-
Notifications
You must be signed in to change notification settings - Fork 5
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
Use ruff to format/lint the codebase #136
Conversation
For #40 |
05db259
to
b0ce41d
Compare
b0ce41d
to
ebbdc93
Compare
.git-blame-ignore-revs
Outdated
@@ -0,0 +1,2 @@ | |||
# Format codebase using ruff @johannaengland 09/09/2024 | |||
07fcc0fa89e719a7cd00d36a3f12e3b552ac6ef2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code is added to this repo by squashing so this will not be correct.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I am aware, I was not planning on merging it like this, I just added this commit to not forget about it later
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This needs splitting into at least two PR's, isort adds much noise. I'll spread this into several direct pushes and then this PR can be rebased to add the github actions.
Btw, have you remembered to set up pre-commit? It's already configured. There are newlines missing from several files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it. the ignore-rev can be fixed by
- squash all non-formatting commits
- format everything in 1 commit
- add that commit to
.git-blame-ignore-revs
- force push and FF-merge to main
Also: next step would be linting/formatting of templates (djlint?) and perhaps js/css? |
|
I have pushed the ruff-ed code to main with the correct hash in .git-blame-ignore-revs. I have not removed the flake8 rule in pre-commit, and I am not linting in pre-commit right now. I would like to see one PR that sets up github actions for this. Let's wait with isort, it has a *very' low priority and I Have Opinions™, I want to play with isort config options and how they interact with ruff before I let it loose on anything. |
We should have a changelog on this somewhere as it is very relevant for developers. I have found two handy flags when using "git blame":
Try it and see with one of the files that has been ruffed. I like what they do, they should be mentioned in the changelog but not in notes. |
It is not possible yet to use the ruff formatter in MegaLinter, therefore I have set up to do the formatting and linting using ruff directly.
This also removes use of flake8 in pre-commit.
When this PR is approved I will squash all the fixing of the linting rules and the formatting to one commit and then add this commit to
.git-blame-ignore-revs
.Also then I can change the workflow to only format/lint the changed files.