-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Allow file: for dependencies in TOML #3255
Conversation
a19b02b
to
ed99a57
Compare
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.
Thank you very much for the very quick implementation @akx.
You went even further than I was expecting and also provided a patch for validate-pyproject
, that is greatly appreciated!
I left some review comments. Some of them are minor, but I think the one about _ensure_previously_set
is important to implement.
Would you also be OK allowing the maintainers to implement changes in the PR? There are minor things/nitpicks that I would edit before accepting the PR, but those are really not worth bothering you.
BTW: sorry for the trouble of requiring you to change the schemas in a different project. My long term vision is to move them to the setuptools repository.
Very great that you managed to find the way of re-generating the validation code even without proper documentation for tox -e generate-validation-code
👏 ❤️
Regarding the acceptance, let's see how Jason review the other PR for setup.cfg
. When that one gets merged, I am very happy to merge the changes here.
edaa15b
to
1de7c5a
Compare
Addressed, thank you!
Honestly I'd prefer if you could make review comments instead, I don't mind doing the work! That'd also give me a bit more insight to the stylistic/... choices for setuptools, for future contributions.
Oh, there was a tox command..? 😁 I just looked at the |
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.
1de7c5a
to
d98528f
Compare
@abravalheri Review comments addressed, docs added! :) |
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.
Thank you very much @akx, it looks very good to me.
I think we can merge this once the other PR is approved.
Any chance you can have a look on the CI to see why is it failing?
d98528f
to
3b86141
Compare
@abravalheri The CI error was just a flake8 complaint about a line length. Fixed that, as well as the |
Summary of changes
This is the pyproject.toml counterpart of #3253, as requested by @abravalheri in #3253 (review).
_validate_pyproject
code was generated using akx/validate-pyproject@61b668fPull Request Checklist
changelog.d/
validate-pyproject
(Allow dependencies/optional-dependencies to use file directives abravalheri/validate-pyproject#37)