-
Notifications
You must be signed in to change notification settings - Fork 571
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
Black tests #1798
Black tests #1798
Conversation
Not sure how everyone feels about this, I find that not having black on tests makes it more likely to get merge conflicts, and most of the test files are more or less black compliant anyway, so this does not make a huge difference. I'm fine if we don't merge this, but would like to consider it. |
@gjhiggins seeing as you work a lot on test it probably makes sense to check with you also, not sure if you find black a nuisance or a benefit. |
Thanks for the consideration, I think black is an extremely useful benefit and makes a significant positive contribution both to reducing the maintenance effort and to supporting a well-founded shared model of the codebase. fwiw, black would reformat 37 files in the
For convenience, I'm including the diff here (having browsed it) because I'm inclined to just let it rip and merge the result. |
Planning to merge this on 2022-04-20. |
@gjhiggins with all the work on tests, I think you are right, I was waiting for #1686 but I don't think it is coming soon, so I'm just going to go ahead with this and we can just rebase all the other things. |
This changes black config to not exclude tests, once we are ready to merge this we can use pre-commit.ci autofix to actually apply black, thereby not having to worry if something more slipped in.
29fc6b9
to
28701db
Compare
pre-commit.ci autofix |
for more information, see https://pre-commit.ci
Summary of changes
This changes black config to not exclude tests, once we are ready to
merge this we can use pre-commit.ci autofix to actually apply black,
thereby not having to worry if something more slipped in.
Checklist
the same change.
so maintainers can fix minor issues and keep your PR up to date.