-
-
Notifications
You must be signed in to change notification settings - Fork 216
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
fix #625: add tomli to install_requires again #628
fix #625: add tomli to install_requires again #628
Conversation
release pipeline is running, will merge after its up |
6.3.1 | ||
===== | ||
|
||
* fix #625: restore tomli in install_requires after the regression changes in took it out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo?
* fix #625: restore tomli in install_requires after the regression changes in took it out | |
* fix #625: restore tomli in install_requires after the regression changes in taking it out |
===== | ||
|
||
* fix #625: restore tomli in install_requires after the regression changes in took it out | ||
and some users never added it even tho they have pyproject.toml files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't frequent various Python lists, so I don't understand why I suddenly have to add "tomli" as a dependency in my pyproject.toml...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wit the change in here you dont, initially one was supposed to use setuptools_scm[toml]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using setuptools_scm[toml]
everywhere here and it's still failing, not sure why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Could be my mistake still somehow, but does show it's tricky to get right)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is transitive, and black is the package breaking. Why it's triggering this at all and not just using a wheel, I don't know.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, so the problem was that tomli was required even when not using toml configuration, so that's what was breaking black. (Again, still don't know why this wasn't getting a wheel and skipping this entirely, but that's why)
I suggest that you at least yank the broken |
new release is up, old release is yanked |
Thank you for your quick fix, this fixed our builds! |
fixes #625