Add Metadata 2.1 #319
Add Metadata 2.1 #319
Conversation
Codecov Report
@@ Coverage Diff @@
## master #319 +/- ##
==========================================
+ Coverage 67.64% 68.51% +0.86%
==========================================
Files 12 12
Lines 578 578
Branches 91 91
==========================================
+ Hits 391 396 +5
+ Misses 161 156 -5
Partials 26 26
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
@di Thank you for this work! I have a few questions and would deeply welcome clarifications (I may well be off-base in all my thinking here).
-
Could I ask you to include a test in this PR? Maybe practically copy your
pkginfomerge request's tests? -
As I was reviewing, I noticed that -- it seems to me -- our
twine/wheel.pyand pkginfo'swheel.pyare fairly duplicative. That's not something to address in this PR but I would appreciate others' thoughts.
| @@ -20,7 +20,7 @@ | |||
|
|
|||
| install_requires = [ | |||
| "tqdm >= 4.14", | |||
| "pkginfo >= 1.0", | |||
| "pkginfo >= 1.4.2", | |||
| "requests >= 2.5.0, != 2.15, != 2.16", | |||
| "requests-toolbelt >= 0.8.0", | |||
| "setuptools >= 0.7.0", | |||
There was a problem hiding this comment.
Do we need to increase our required setuptools version to something like setuptools > v38.5.2 (the current version as far as I can tell) to ensure we capture the update in pypa/setuptools#1286 ?
There was a problem hiding this comment.
Or rather, even if we don't need to since the new additional metadata is optional, should we take this opportunity to do some kind of required version increase regardless?
There was a problem hiding this comment.
The only reason we have setuptools listed in the requirements is so that we can reliably get it's version when running twine --version.
Users with older versions of setuptools will still be able to upload their distributions with twine, they just won't be able to create distributions with Metadata 2.1 fields. If they want to do that, they should upgrade their setuptools (it's not twine's responsibility to make that decision for them).
There was a problem hiding this comment.
Totally understand. Thanks. So, if we choose to do any user-facing messaging about this feature &, for instance, Markdown support, we should indicate that such-and-such setuptools is a requirement.
| @@ -148,6 +148,10 @@ def metadata_dictionary(self): | |||
| "requires_dist": meta.requires_dist, | |||
| "requires_external": meta.requires_external, | |||
| "requires_python": meta.requires_python, | |||
|
|
|||
| # Metadata 2.1 | |||
| "provides_extras": meta.provides_extras, | |||
There was a problem hiding this comment.
Looks right to me per the names for these attributes as provided by the class from pkginfo.
Let me make sure I've got this right: even though the field defined in the PEP is Provides-Extra, we use the "s" at the end to indicate that it might be multiple strings, as with project_urls. Right?
There was a problem hiding this comment.
Yes, that seems to be the status quo (really, to indicate that this is a list).
| @@ -11,7 +11,7 @@ requires-dist = | |||
| tqdm >= 4.14 | |||
| requests >= 2.5.0, != 2.15, != 2.16 | |||
| requests-toolbelt >= 0.8.0 | |||
| pkginfo >= 1.0 | |||
| pkginfo >= 1.4.2 | |||
| setuptools >= 0.7.0 | |||
There was a problem hiding this comment.
Same question here as with setup.py about setuptools versions.
There was a problem hiding this comment.
Same answer as above.
|
@di Please also add a line about yourself to that would be great -- if not, I can do it. |
I added a test, I'm not a huge fan of it but it should increase our coverage a bit so can't hurt. Also addressed everything else I think. |
There was a problem hiding this comment.
I created a virtualenv, installed @di's branches of twine & setuptools, added a description_content_type to the setup.py, and created and uploaded a package to Test PyPI successfully. Yay!
|
There's now a release candidate for the next version of Twine available containing this PR. 🎉 |
This PR updates twine to upload new fields defined in PEP 566.