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

Format the Wagtail codebase with black #6056

Closed
thibaudcolas opened this issue May 22, 2020 · 5 comments
Closed

Format the Wagtail codebase with black #6056

thibaudcolas opened this issue May 22, 2020 · 5 comments
Assignees
Milestone

Comments

@thibaudcolas
Copy link
Member

thibaudcolas commented May 22, 2020

We’ve discussed reformatting Wagtail’s codebase with black multiple times in the past, but it seems there is no public record of those discussions / why we haven’t done it yet but intend to.

Here is what we discussed in a past core team meeting:

TL;DR; wait until Black 1.0, and Django uses it


Relevant links:


For reference, we’ve also discussed using Prettier as well (for JS, SCSS, Markdown, JSON, YAML). Here I think it’s just a matter of someone putting a small proposal together (in a new issue), and then making the change – we’re all in agreement we’d love to see this happen.

@thibaudcolas thibaudcolas added this to the some-day milestone May 22, 2020
@thibaudcolas thibaudcolas changed the title Reformat the Wagtail codebase with black Format the Wagtail codebase with black May 22, 2020
@yhoiseth
Copy link
Contributor

yhoiseth commented May 22, 2020

@thibaudcolas Thanks for writing this up 🙂

For reference, we’ve also discussed using Prettier as well (for JS, SCSS, Markdown, JSON, YAML). Here I think it’s just a matter of someone putting a small proposal together (in a new issue), and then making the change – we’re all in agreement we’d love to see this happen.

I can give it a go.

@ichard26
Copy link

ichard26 commented Jul 6, 2020

Black contributor here, I've stumbled across this issue by chance :P FYI, it is possible ignore certain commits when calling git blame using --ignore-rev or --ignore-revs-file since Git version 2.23. So at least that part of the git history won't be ruined, although the GitHub blame UI sadly doesn't support this feature yet.

For more information:
https://github.com/psf/black#migrating-your-code-style-without-ruining-git-blame
https://git-scm.com/docs/git-blame#Documentation/git-blame.txt---ignore-revltrevgt

@thibaudcolas
Copy link
Member Author

FYI for people interested in this, based on psf/black#2529. The info isn’t where I’d have expected this but the FAQ explicitly says:

[…] the first stable release, expected for January 2022

cc @Tijani-Dia who was interested in working on this.

@thibaudcolas
Copy link
Member Author

thibaudcolas commented Feb 1, 2022

Black has now made a stable release, so if anyone wants to pick this up please go ahead. I’d like us to proceed with this similarly with how we’re adding Prettier:


Based on reviewing how Wagtail is set up right now, I’d expect the black setup to require:

  • A pyproject.toml with the configuration. Potentially a few changes to our .editorconfig too (unsure).
  • black installed via our setup.py testing_extras like other static analysis tools
  • Running in CircleCI in our config.yml’s backend job like for flake8
  • Adding to our Makefile’s lint target
  • Updating to the latest isort with the black-compatible profile
  • Potentially updating our flake8 rules to be compatible with black formatting
  • Docs updates to python_guidelines.rst to mention black
  • A black hook in our .pre-commit-config.yaml configuration

I’ll mark this as a good first issue – the steps involved are by no means simple, but this none of this requires extensive knowledge of how Wagtail works.

@thibaudcolas thibaudcolas modified the milestones: some-day, 2.17 Feb 1, 2022
@zerolab zerolab self-assigned this Feb 9, 2022
@zerolab zerolab mentioned this issue Feb 11, 2022
2 tasks
@thibaudcolas
Copy link
Member Author

Done by Dan in #7967!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants