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

fmt for black 22.12 #5295

Merged
merged 2 commits into from
Dec 27, 2022
Merged

fmt for black 22.12 #5295

merged 2 commits into from
Dec 27, 2022

Conversation

rorour
Copy link
Contributor

@rorour rorour commented Dec 27, 2022

@rorour rorour requested a review from robnagler December 27, 2022 04:20
@robnagler
Copy link
Member

I thought the difference was between 22.12 and 23.x?

@rorour
Copy link
Contributor Author

rorour commented Dec 27, 2022

What version of black have you been using? Before any of these changes I was running 22.6. I assume we would all have to be on the same or very close. I think we have just been lucky to not run into issues with black versions so far.

When working on the ci changes for pykern I discovered a bug that was patched in 22.12, and this pr contains the changes to match fmt with 22.12.

The reason for the fixed version required in pykern and the reason I pointed out the difference from 23.x would be future proofing. Not sure if that's the right term but someone with 23.x vs 22.12 would have different outputs for fmt which would cause issues passing ci.

@robnagler
Copy link
Member

This is a real problem, because we can't have battling formatter versions. Our code base will be a mess. This is not something I would have anticipated when "buying" black. I thought they would have figured all this out already, and be done with the formatting changes. The definition of "black" changes from year to year. I wonder if we wait until the new year that we'll have a 23.x version that will be stable all year.

We should require a fixed version for this reason. Thank you @rorour for pointing that out. I don't know if it is future proofing as much as chaos proofing.

In any event, we should fix the version with black~=22.0 so we get bug fixes within the year. We'll have to figure out how to deal with broken PRs on Sirepo, because the version change has to coincide with the sirepo-ci image change.

@robnagler robnagler merged commit f16381b into master Dec 27, 2022
@robnagler robnagler deleted the fmt-black-update branch December 27, 2022 23:34
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.

2 participants