-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PEP 771: Default Extras for Python Software Packages #4198
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
I confirm that I am sponsoring this PEP. |
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.
Very nice PEP! LGTM in general although I have included some comments and suggestions.
@warsaw - thanks for the review! I've now pushed changes to address your comments. |
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.
Thanks for writing this up! It sounds useful.
@warsaw - thank you for the review! I believe I have implemented all your comments. In addition, I have pushed a change to the reference implementation section to update links there, as the work on this implementation is now fully functional. |
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.
A few small typos
This comment was marked as outdated.
This comment was marked as outdated.
Following a suggestion by @pradyunsg, it might be easiest to no longer make any substantive updates to this PR, and instead publish it once/if the PEP editors feel it is ready. Once it is published, I will shortly after open a PR which makes some changes/additions based on the discussion in this thread: https://discuss.python.org/t/pre-publish-pep-711-default-extras-for-python-software-packages/77892 It will then be easier to review these changes rather than having to re-read the whole PEP, which is getting long. And once that follow-up PR is merged, we can open the main discussion thread for the PEP. |
fe5b74d
to
e834905
Compare
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Hugo van Kemenade <[email protected]>
Co-authored-by: Dustin Ingram <[email protected]>
Co-authored-by: Paul Moore <[email protected]>
…mention that Metadata-Version will need to be bumped
e834905
to
9927fc7
Compare
I have rebased due to a conflict in |
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.
Thanks for the update! Looks good editorially, let's merge later today if there's no last minute suggestions from others.
PS Generally we try and avoid force-pushing, it's easier to review what's changed with normal pushes, and everything will be squash-merged to a single commit at the end anyway.
Another tip: please avoid putting @usernames
in commit messages, GitHub can be quite annoying and end up pinging several times from other forks :)
@hugovk oops, sorry about the force push, there was a conflict and I am used to rebasing in this case for other projects. |
Thank you! Next, please post to discuss.python.org, then open a quick PR to put that URL in |
@hugovk - thanks for merging! As mentioned in #4198 (comment) I'd like to make a follow-up PR with a few key changes based on the pre-publish discuss thread, and only once that is merged open the main discussion thread. Would that be ok? (I plan to open up the follow-up PR this evening or tomorrow morning) |
This seems fine, there's no rush to open the next A |
Basic requirements (all PEP Types)
pep-NNNN.rst
), PR title (PEP 123: <Title of PEP>
) andPEP
headerAuthor
orSponsor
, and formally confirmed their approvalAuthor
,Status
(Draft
),Type
andCreated
headers filled out correctlyPEP-Delegate
,Topic
,Requires
andReplaces
headers completed if appropriate.github/CODEOWNERS
for the PEPStandards Track requirements
Python-Version
set to valid (pre-beta) future Python version, if relevantDiscussions-To
andPost-History
Question for PEP editors: there are some gaps in the PEP numbering, so is this the correct number to pick? I'm happy to update this if not.
📚 Documentation preview 📚: https://pep-previews--4198.org.readthedocs.build/pep-0771/