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

build(deps): update black to v23.9.1 #1000

Merged
merged 6 commits into from
Oct 9, 2023
Merged

build(deps): update black to v23.9.1 #1000

merged 6 commits into from
Oct 9, 2023

Conversation

onerandomusername
Copy link
Member

Summary

Bump black to 23.3.0 and enable their preview style which contains new fixes. Their stability policy has new changes released once a year, but a lot of these changes made the code much more readable.

This moves us closer to being able to enable E501 (if we want), as a multitude of these changes move long strings onto multiple lines.

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

@onerandomusername onerandomusername added s: needs review Issue/PR is awaiting reviews t: dependencies Addition/update/removal of dependencies t: refactor/typing/lint Refactors, typing changes and/or linting changes skip news labels Apr 6, 2023
@shiftinv
Copy link
Member

shiftinv commented Apr 7, 2023

Overall this seems fine, I'll have a more detailed look tomorrow. I'm not really a huge fan of specific parts of string splitting - it's a bit too opinionated imo - but I know we don't get to pick and choose in the end.

Black doesn't "equally" split strings, i.e. such that the resulting lines are approximately equal in length, which looks rather weird:

# input
"View is not persistent. Items need to have a custom_id set and View must have no timeout"

# current output
"View is not persistent. Items need to have a custom_id set and View must have no"
" timeout"

This can be fixed through manual review (black doesn't reformat literals that are already within line length limits), but it's not ideal.

The preview style still has a couple issues with multiline strings in specific contexts, however we seemingly haven't hit them (yet), to be fair. Another thing is visual misalignment of multiline f-strings, as outlined here: psf/black#2915 (comment)

@shiftinv
Copy link
Member

shiftinv commented Oct 4, 2023

While some of these changes are neat, updating this PR from black v23.3.0 to v23.9.1 adds another ~1k lines of changes, some of which partially revert changes originally made by v23.3.0.
The preview style is unstable by nature (that's the entire point, after all), but I don't think this is sustainable for us... let's wait for a v24.x, maybe?

@shiftinv shiftinv removed s: needs review Issue/PR is awaiting reviews t: refactor/typing/lint Refactors, typing changes and/or linting changes labels Oct 9, 2023
@shiftinv shiftinv added this to the disnake v2.10 milestone Oct 9, 2023
@shiftinv
Copy link
Member

shiftinv commented Oct 9, 2023

Changed the scope of this PR to only update black to 23.9.1 for now, which we need to be able to add target-version = [..., "py312"] in the future.
This now also uses mypyc-compiled black in pre-commit (which is supposedly ~2x faster), recently documented here.

@shiftinv shiftinv changed the title chore(lint): enable black's preview style build(deps): update black to v23.9.1 Oct 9, 2023
@shiftinv shiftinv merged commit 3b604f6 into master Oct 9, 2023
27 checks passed
@shiftinv shiftinv deleted the chore/black-preview branch October 9, 2023 22:31
@onerandomusername onerandomusername restored the chore/black-preview branch October 9, 2023 22:35
@onerandomusername onerandomusername deleted the chore/black-preview branch October 9, 2023 22:36
@onerandomusername
Copy link
Member Author

lgtm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news t: dependencies Addition/update/removal of dependencies
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants