-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add djade linter for Django templates (Fixes #2372) #2382
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
base: main
Are you sure you want to change the base?
Conversation
cb91ead to
892f574
Compare
sarahboyce
left a comment
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.
Thank you! A couple of minor nitpicks
sarahboyce
left a comment
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.
Looks good to me!
|
Thanks for the review and approval, @sarahboyce! I appreciate the guidance on the configuration details. I’ll leave the commits as they are for now, assuming they will be squashed upon merging. Ready when you are! |
|
I'll ask for a rebase from the latest main (not a merge in) and resolution of conflicts. Other than that, looks awesome. |
1bbda29 to
0eb5c36
Compare
|
@ulgens Done! I've rebased on main and resolved the conflicts in doc.html.I also verified that the linter passes locally. Ready for merge! |
|
@aNsHuL5217 Thank you. Adding my approval, considering the PR will be squash merged. Adding the mergers team for review. |
sarahboyce
left a comment
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.
Can you resolve the conflicts and rerun thr linter based off Django 6.0?
Co-authored-by: Sarah Boyce <[email protected]>
Co-authored-by: Sarah Boyce <[email protected]>
0eb5c36 to
aefc843
Compare
|
@sarahboyce Done! I've rebased on main, updated djade to 1.7.0, and set the target version to 6.0. The linter caught a few new changes with the upgrade. Ready for review! |
sarahboyce
left a comment
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.
Thank you for the updates! I have a couple of questions which I think will have come from resolving conflicts
Co-authored-by: Sarah Boyce <[email protected]>
f9b9a81 to
159a6c1
Compare
|
@sarahboyce Fixed! I reset |
Summary
This PR adds the
djadelinter to the project's pre-commit configuration and applies formatting fixes to existing Django templates. This ensures consistent code style (e.g., standardizing whitespace in{{ variables }}) across the codebase. Fixes #2372Rationale
As discussed in #2372,
djadewas selected over alternatives likedjLintbecausedjadeis actively maintained and specifically targets Django template formatting. This aligns with the project's goal of using stable, long-term tools.Changes
adamchainz/djade-pre-commit(v1.6.0) to.pre-commit-config.yaml..htmltemplates.Verification
pre-commit run djade --all-fileslocally (passed formatting checks).