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

Don't skip magic-trailing-comma [black] #13418

Closed
wants to merge 3 commits into from

Conversation

cdce8p
Copy link
Collaborator

@cdce8p cdce8p commented Aug 15, 2022

Description

Follow up to discussion in #13412 (comment)

Remove black setting to skip-maagic-trailing-comma since in some instances it can result in better readable code. With the setting enabled, black will always choose the most compact function definition even if it isn't the best solution.
https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#the-magic-trailing-comma

Initial comment adding it: #12424 (comment)

Test Plan

I've reapplied the changes suggested in #13412 (review) to show the impact of the setting change.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Aug 15, 2022

Thanks, but I thought I was pretty clear in #13412 (comment)

I'll leave this open for a couple days before closing, in case you get luck with a kinder maintainer than I :-)

@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@JelleZijlstra
Copy link
Member

I'd also prefer to keep the current configuration. I only begrudgingly accepted the trailing comma going into Black in the first place: psf/black#826 (comment).

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 15, 2022

I'm not a maintainer here, but FWIW, I also prefer the current configuration.

@cdce8p
Copy link
Collaborator Author

cdce8p commented Aug 15, 2022

Ok. Seems like most prefer the current look. I'll go ahead and close this then.

@cdce8p cdce8p closed this Aug 15, 2022
@cdce8p cdce8p deleted the black-magic-comma branch August 15, 2022 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants