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

Changes in Black internals may break Darker #186

Closed
akaihola opened this issue Sep 2, 2021 · 5 comments · Fixed by #284
Closed

Changes in Black internals may break Darker #186

akaihola opened this issue Sep 2, 2021 · 5 comments · Fixed by #284
Labels
Milestone

Comments

@akaihola
Copy link
Owner

akaihola commented Sep 2, 2021

We're using Black's Python API directly. Black doesn't yet define a public API (see psf/black#779), so changes in Black may break Darker. This has in fact already happened at least twice.

To get an early warning about such changes, we could add a GitHub Action workflow for testing against Black main instead of its latest release.

@akaihola akaihola added the CI label Sep 2, 2021
@akaihola akaihola added this to the 1.3.1 milestone Sep 2, 2021
@akaihola
Copy link
Owner Author

akaihola commented Sep 4, 2021

@flying-sheep, you may be interested in this – we just fixed one incompatibility in #189.

@akaihola akaihola modified the milestones: 1.3.1, 1.3.2 Sep 4, 2021
@akaihola akaihola modified the milestones: 1.3.2, 1.4.0, 1.4.1 Oct 28, 2021
@akaihola
Copy link
Owner Author

akaihola commented Feb 10, 2022

Changes in Black internals just broke Darker, see #270 (and earlier contributed patches in #267, #268 and #269; thanks @jabesq, @yoursvivek and @Mystic-Mirage).

Some post-mortem brainstorming might help with coming up with ideas to protect against this.

@yoursvivek
Copy link
Collaborator

I would say, there was a gap in version check with-in this project itself. Black upgraded a major version bump ("black==22.0") and are allowed to do breaking change as per any versioning guidelines.

Darker should limit black to minor version compatibility check. Moving forward using black~=22.0. I do acknowledge that darker would need more releases to bump black version but better than broken package.

@akaihola
Copy link
Owner Author

Hi @yoursvivek, thanks for pointing this out!

But isn't 22.1 the first non-beta-numbered release from Black? As long as the version number looked similar to 21.1b1 I always assumed no compatibility guarantees from Black anyway.

Also, Black's version numbers look like year/month based and not SemVer, so are you sure they would now delay any non-breaking changes until 23.1?

I've also learned that Black doesn't actually commit to any kind of a public Python API. So we're using Black internals anyway. See psf/black#779 for more information.

Of all the options I can think of, I like most the idea to add one more automated test environment which installs master / main checkouts of dependencies (esp. Black) instead of latest releases. This way we'd get an early warning of new incompatibilities and could make a compatibility release even before the dependency publishes the compatibility breaking release.

@yoursvivek
Copy link
Collaborator

so are you sure they would now delay any non-breaking changes until 23.1?

Well, I'm not. Just believed so in good faith.

Of all the options I can think of, I like most the idea to add one more automated test environment which installs master / main checkouts of dependencies (esp. Black) instead of latest releases. This way we'd get an early warning of new incompatibilities and could make a compatibility release even before the dependency publishes the compatibility breaking release.

This sounds more resilient plan.

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

Successfully merging a pull request may close this issue.

2 participants