PEP NNN: Recording provenance of installed packages #2988
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
You can take PEP 705. Please add the file to |
b506be8 to
6603971
Compare
6603971 to
9218129
Compare
9218129 to
b040c1d
Compare
Removed based on @pfmoore request. |
|
Who's the sponsor for this PEP? |
|
The initial version had @pfmoore as the sponsor, but I see that's now been removed. If there is no sponsor, the PEP cannot be accepted into the repo. |
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
0c5ba0c to
2a71e6e
Compare
There was a problem hiding this comment.
Standard reminder: You can apply the suggestions you want in one go with Files changed -> Add to batch -> Commit
While I haven't analyzed it in detail, it seems this proposal may be better off being discussed and refined first as a pre-PEP first before submitting it as a formal PEP, as at least skimming it, it seems a little premature to publish as a PEP missing nearly all the standard sections, most notably the actual specification. Without that, I'm not sure it is complete enough yet to be considered a PEP (and cannot be accepted without a formal Sponsor and the other various requirements below being met).
As such, and due to limited time, I haven't reviewed the PEP prose text in detail beyond basic rendering defects and the checklist below.
However, it would really use some proofreading, as it has many typos, grammar errors, word order issues, and missing definite articles ("the") required in English ("record the provenance", "in the form of a JSON file", "the .dist-info directory", etc., just in the first paragraph of the abstract), some of
8000
which Hugo pointed out.
Basic requirements (all PEP Types)
- Read and followed PEP 1 & PEP 12
- File created from the latest PEP template
- PEP has next available number, & set in filename (
pep-NNNN.rst), PR title (PEP 123: <Title of PEP>) andPEPheader - Title clearly, accurately and concisely describes the content in 79 characters or less
-
PEP,Title,Author,Status(Draft),TypeandCreatedheaders filled out correctly -
PEP-Delegate,Topic,RequiresandReplacesheaders completed if appropriate - Core dev/PEP editor listed as author or sponsor, and formally confirmed their approval
- Required sections included
- Abstract (first section)
- Copyright (last section; exact wording from template required)
- Code is well-formatted (PEP 7/PEP 8) and is in code blocks, with the right lexer names if non-Python
- PEP builds with no warnings, pre-commit checks pass and content displays as intended in the rendered HTML
- Authors/sponsor added to
.github/CODEOWNERSfor the PEP
Standards Track requirements
- PEP topic discussed in a suitable venue with general agreement that a PEP is appropriate
- Suggested sections included (unless not applicable)
- Motivation
- Rationale
- Specification
- Backwards Compatibility
- Security Implications
- How to Teach This
- Reference Implementation
- Rejected Ideas
- Open Issues
-
Python-Versionset to valid (pre-beta) future Python version -
Any project stated in the PEP as supporting/endorsing/benefiting from it confirms suchN/A - Right before or after initial merging, PEP discussion thread created and linked to in
Discussions-ToandPost-History
| Title: Recording provenance of installed packages | ||
| Author: Fridolin Pokorny <fridolin.pokorny at gmail.com>, Trishank Karthik Kuppusamy <karthik@trishank.com> | ||
| Sponsor: | ||
| PEP-Delegate: |
There was a problem hiding this comment.
| PEP-Delegate: | |
| PEP-Delegate: Paul Moore <p.f.moore@gmail.com> |
@pfmoore , as the standing PEP Delegate for packaging metadata PEPs, are you going to assume this role (assuming this is accepted as a PEP with the appropriate discussion and reworking), or would we need to look for someone else (or submit to the SC)?
Alternatively, if no PEP delegate is assigned for now, then this header should simply be omitted
| PEP-Delegate: |
| Examples | ||
| ======== |
There was a problem hiding this comment.
You've jumped right to the examples before introducing any actual specification (and I don't see a section titled such anywhere). I would suggest starting with a clear, precise and unambiguous specification first, which is a preliminary requirement to be able to accept your PEP.
pep-0705.rst
Outdated
| In both cases, the JSON document is stating the following entries: | ||
|
|
||
| * ``archive_info.hash`` MUST be present with a value of ``<hash-algorithm>=<expected-hash 8000 >``, | ||
| currently supported hash algorithm is only ``sha256``. | ||
|
|
||
| * ``url`` - MUST be present and points to source from where the package was obtained. | ||
| The value MUST be stripped of any sensitive authentication information, for security | ||
| reasons. |
There was a problem hiding this comment.
This, and perhaps a few other bits, seems to be the closest thing I can find to a concrete specification, and should therefore presumably go in a Specification section prior to the Examples.
|
It seems the review comments were marked resolved, but few if any of them appear to actually be resolved. Also, FYI, while occasionally appropriate while the PR is still in draft state, before it has seen significant review, to fixup minor issues immediately after pushing a commit (e.g. failing CI checks), or extraordinary circumstances, rewriting history and rebasing changes in to a PR under active review instead of pushing new distinct commits is often frowned upon here, and can make it much more difficult for reviewers to clearly see what's changed, among other issues. @fridex , FYI, you can apply some or all suggestions easily in just a couple clicks by going to |
|
I would actually suggest to close this PR until a sponsor has come forward. As CAM points out, the draft is also missing many important sections like the Specification. I would suggest working on that separately first with your sponsor before submitting to the peps repo. |
|
@CAM-Gerlach @JelleZijlstra @hugovk thanks for your suggestions and sorry if PEP was submitted prematurely.
Sorry for my missing knowledge here - what would be the best way to find a sponsor? |
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM> Co-authored-by: Hugo van Kemenade <hugovk@users.noreply.github.com>
|
@fridex maybe someone will come forward on the Discuss thread. I posted there to understand Paul's position better for now. |
|
I've ticked off a couple more boxes in the checklist, but as we've mentioned, it still needs some pretty fundamental rewrites and additions to be considered for prelimi A93C nary acceptance as a draft PEP (along with a sponsor, of course). You could consider closing this for now, iterating in the branch on your fork via a discussion thread, which is also a great venue to find a sponsor, and then once it's ready to be reconsidered, re-opening this PR or a new one. It's possible that Dustin, Donald or one of the other PyPI people might be interested in sponsoring this or at least giving feedback—you could reach out to them. Theoretically, as a PEP editor and packaging community member I could sponsor it too, but I would need to see something a lot closer to complete to be able to consider that, and right now with everything I have on my plate online and off, I don't think I can commit to dedicating the time it would take to help you get to that point myself, unfortuantely. |
|
Thanks all for review and work on this - it really looks like the PR was opened too soon - my sincere apologies for it. Closing for now until a sponsor is found and more polished version is available. Thanks again. |
|
PEP Editor note: because this was closed, we re-allocated PEP 705 to #2997. We can give this one a new number when a sponsor is found. Thanks! |
|
Just in case anyone was following this, after considerable community discussion and revision, this was re-opened as #3076 and assigned PEP 710. |
Signed-off-by: Fridolin Pokorny fridolin.pokorny@datadoghq.com