Skip to content

gh-93243: Make smtpd private before porting its users - #93246

Merged
miss-islington merged 19 commits into
python:mainfrom
arhadthedev:privatize-smtpd
Aug 6, 2022
Merged

gh-93243: Make smtpd private before porting its users#93246
miss-islington merged 19 commits into
python:mainfrom
arhadthedev:privatize-smtpd

Conversation

@arhadthedev

@arhadthedevarhadthedev commented May 26, 2022

Copy link
Copy Markdown
Member

gh-93243

This PR is required to reduce diffs of the following porting (no need to either maintain documentation and tests consistent with each porting step, or try to port everything and remove smtpd in a single PR).

Automerge-Triggered-By: GH:warsaw

Comment threadPCbuild/lib.pyproj

@AA-TurnerAA-Turner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some docs/wording comments:

A

Comment threadDoc/whatsnew/3.12.rst Outdated
Comment threadDoc/whatsnew/3.12.rst Outdated
Comment threadLib/test/mock_socket.py Outdated
Comment threadLib/test/mock_socket.py Outdated
Comment threadMisc/NEWS.d/next/Library/2022-05-26-08-41-34.gh-issue-93243.uw6x5z.rst Outdated
arhadthedevand others added 4 commits June 4, 2022 19:01
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Comment threadDoc/whatsnew/3.12.rst Outdated
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@hugovk

Copy link
Copy Markdown
Member

cc also PEP-594 co-author @brettcannon.

@brettcannonbrettcannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably need logging and email maintainers to sign off that they are okay with keeping smptd around for testing.

Comment threadLib/test/test_logging.py
@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@arhadthedev

Copy link
Copy Markdown
MemberAuthor

I have made the requested changes; please review again.

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@brettcannon: please review the changes made to this pull request.

@arhadthedev

arhadthedev commented Jul 29, 2022

Copy link
Copy Markdown
MemberAuthor

Probably need logging and email maintainers to sign off that they are okay with keeping smptd around for testing.

@vsajip@warsaw could you review and possibly merge this PR please? It deletes smtpd tests in advance and, for consistency, makes the module private by moving it into Lib/test/smtpd.py.

This is necessary to allow further reviewable-sized PRs (one for each SMTP* class moved into its consumer in test) without massive changes of Lib/test/test_smtpd.py for synchronization.

@warsawwarsaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a small rewording request.

Comment threadDoc/whatsnew/3.12.rst Outdated
@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@arhadthedev

Copy link
Copy Markdown
MemberAuthor

I have made the requested changes; please review again.

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@brettcannon, @warsaw: please review the changes made to this pull request.

Wording suggestion.
@brettcannon

Copy link
Copy Markdown
Member

@warsaw not sure if this is showing up in your review queue as the GH UI is not letting me re-request your review.

Small reword suggestion.
@warsaw

Copy link
Copy Markdown
Member

Thanks for the ping @brettcannon - I'm good with this, with my one edit to the source branch. I thought I'd pushed that through previously, but upon further review, it doesn't look like it stuck. Trying that again...

@warsawwarsaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this time it looks like my small wording edit got committed. Thanks for the contribution @arhadthedev ! Approving and setting the automerge label.

@miss-islington

Copy link
Copy Markdown
Contributor

Status check is done, and it's a success ✅ .

@miss-islington
miss-islington merged commit 56d16e8 into python:mainAug 6, 2022
@arhadthedev
arhadthedev deleted the privatize-smtpd branch August 7, 2022 06:55
iritkatriel pushed a commit to iritkatriel/cpython that referenced this pull request Aug 11, 2022
…-93246)
pythongh-93243
This PR is required to reduce diffs of the following porting (no need to either maintain documentation and tests consistent with each porting step, or try to port everything and remove smtpd in a single PR).
Automerge-Triggered-By: GH:warsaw
@hugovkhugovk mentioned this pull request Oct 28, 2022
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@arhadthedev@hugovk@bedevere-bot@brettcannon@warsaw@miss-islington@AA-Turner