-
- Notifications
You must be signed in to change notification settings - Fork 1.3k
Allow file: for dependencies in TOML #3255
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
Conversation
a19b02b to ed99a57 Compare
abravalheri left a comment
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 |
abravalheri left a comment
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! :) |
abravalheri left a comment
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_pyprojectcode 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)