PEP 710: Recording the provenance of installed packages #3076
Conversation
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
|
PEP 710 is available. |
Thanks, renamed to PEP-710. |
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
|
Please include an update to |
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
There was a problem hiding this comment.
Standard reminder: You can directly apply all the suggestions you want in one go by going to Files changed -> Clicking Add to batch on each suggestion -> When done, clicking Commit
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
- Core dev/PEP editor listed as
AuthororSponsor, and formally confirmed their approval -
Author,Status(Draft),TypeandCreatedheaders filled out correctly -
PEP-Delegate,Topic,RequiresandReplacesheaders completed if appropriate - 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
- Right before or after initial merging, PEP discussion thread created and linked to in
Discussions-ToandPost-History
There was a problem hiding this comment.
FWIW, this is pretty huge improvement over last time; thanks @fridex and everyone who contributed! Other than a grab-bag of requested fixes (nearly all one-click suggestions), I only have a handful of higher-level comments/missing pieces, most if not all of which should be fairly straightforward to add.
Higher-level comments for after suggestions are applied:
- I suggest include a brief Rationale section describing the reasoning behind the approach chosen in this PEP, and/or a brief history of the discussions that led to this PEP being proposed
- A security implication section should at least briefly mention the security benefits to this proposal, and any potential issues it may have
- A one-line "How to teach this" section could simply state that this metadata is intended for tools and is not directly visible to end users.
- A Reference Implementation section should highlight and link the PRs you've already prepared (sort of similar to what you previously put in the "References" section instead
- By standard convention, Backward Compatibility goes directly after the specification, not at the very end after Rejected Ideas and Open Issues
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Minor formatting fixes
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
|
Just for the others' benefit (and to cross-reference them), this was originally submitted as PR #2988 , but later withdrawn, revised and re-submitted here. |
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
There was a problem hiding this comment.
Great job adding a detailed and complete Rationale section; thanks! I have some textual copyedits, fixes and refinements on that, but otherwise this is looking pretty good to go on my end, pending final review and approval from @dstufft and any other subject-matter experts. Thanks!
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Thanks for suggestions! I planned to include the mentioned research on possible clash with EDIT: done in de7cf45 |
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
There was a problem hiding this comment.
Various suggestions on the added content (many related to removing an unnecessarily-complex layer of link indirection, as well as some grammar and other textual fixes), as well as a couple broader comments—most notably, I recommend moving the comprehensive tool survey to an appendix (as described in more detail in a comment below). Thanks!
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Signed-off-by: Fridolin Pokorny <fridolin.pokorny@datadoghq.com>
|
Given that this is being extensively reviewed, wanna mark this as ready for review? ;) |
There was a problem hiding this comment.
Just a couple last small syntax fixes (as suggestions); otherwise LGTM from me! Thanks so much for your patience and hard work on this, @fridex !
Also, just BTW—after much testing, I was able to isolate an edge-case issue with our code where the References section in your PEP wasn't being properly hidden, due to containing reference names that duplicate/shadow implicit section title references (which isn't a problem, so long as you don't use them...one more reason not to). I've submitted PR #3083 to take care of that (which isn't a blocker to merging this one).
Co-authored-by: C.A.M. Gerlach <CAM.Gerlach@Gerlach.CAM>
Thanks to you, the PEP is on a whole different level! Thank you very much for your time, suggestions and fixes. |
You're most welcome! But to be fair, I'd say there was a many-level difference between when the PEP was first submitted and now, which was thanks to your work and that of the others who advised you on it. So, thank you! |
|
Process Note: I can sync up with @fridex this week to merge this and update the post history and make the post. |
Definitely, it was great collaboration effort so far! I tried to capture everyone who contributed in some way in the Acknowledgements section of the PEP -- if there is anyone missing, please don't hesitate to add her/him. Thanks to all! (BTW it is sad the Acknowledgement section is one of the last ones in the PEP template.) |
| ------ | ||
|
|
||
| The installation logic in `Poetry`_ depends on the | ||
| ``installer.modern-installer`` configuration option (`see docs |
Based on discussion in this thread, opening this pull-request with a draft PEP.
📚 Documentation preview 📚: https://pep-previews--3076.org.readthedocs.build/