GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-93243: Make smtpd private before porting its users by arhadthedev · Pull Request #93246 · python/cpython

Conversation

@arhadthedev

@arhadthedev arhadthedev commented May 26, 2022 •
edited by miss-islington
Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@arhadthedev
pythongh-93243: Make smtpd private to port its users
021e85c
@arhadthedev
arhadthedev requested review from a team and vsajip as code owners May 26, 2022 06:49
@bedevere-bot bedevere-bot added the awaiting review label May 26, 2022
hugovk
hugovk reviewed Jun 4, 2022
View reviewed changes
Comment thread PCbuild/lib.pyproj Resolved

Uh oh!

There was an error while loading. Please reload this page.
arhadthedev added 4 commits June 4, 2022 16:24
@arhadthedev
Merge branch 'main' into privatize-smtpd
ed21b2b
@arhadthedev
Move smtpd to its only user for further porting to asyncio
18a5615
@arhadthedev
Point test_logging to the new smtpd location too
6f2c565
@arhadthedev
Remove smptd from standalone module lists
f5aee16
AA-Turner
AA-Turner reviewed Jun 4, 2022
View reviewed changes

@AA-Turner AA-Turner left a comment

Copy link
Copy Markdown
Member

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.

Some docs/wording comments:

A

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/mock_socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Lib/test/mock_socket.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Library/2022-05-26-08-41-34.gh-issue-93243.uw6x5z.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
arhadthedev and others added 4 commits June 4, 2022 19:01
@arhadthedev @AA-Turner
Update Doc/whatsnew/3.12.rst
c7cb1e3 
Co-authored-by: Adam Turner <[email protected]>
@arhadthedev @AA-Turner
Update Doc/whatsnew/3.12.rst
8180429 
Co-authored-by: Adam Turner <[email protected]>
@arhadthedev @AA-Turner
Address the review of AA-Turner (using suggestion batching)
b70bf5a 
Co-authored-by: Adam Turner <[email protected]>
@arhadthedev
Remove non-working :mod:
4054ac2
AA-Turner
AA-Turner reviewed Jun 4, 2022
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev @AA-Turner
Update Doc/whatsnew/3.12.rst
eb6ad56 
Co-authored-by: Adam Turner <[email protected]>
@hugovk

hugovk commented Jun 4, 2022

Copy link
Copy Markdown
Member

cc also PEP-594 co-author @brettcannon.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
brettcannon
brettcannon requested changes Jun 8, 2022
View reviewed changes

@brettcannon brettcannon left a comment

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_logging.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting review label Jun 8, 2022
@bedevere-bot

bedevere-bot commented Jun 8, 2022

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting changes label Jun 8, 2022
@arhadthedev
Address @brettcannon's review
133735a
@arhadthedev

arhadthedev commented Jun 8, 2022

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Jun 8, 2022
@bedevere-bot

bedevere-bot commented Jun 8, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from brettcannon June 8, 2022 21:17
arhadthedev added 2 commits June 23, 2022 23:43
@arhadthedev
Merge branch 'main' into privatize-smtpd
38fd57b
@arhadthedev
Merge branch 'main' into privatize-smtpd
1f839b9
@arhadthedev

arhadthedev commented Jul 29, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev
Merge branch 'main' into privatize-smtpd
7423617
warsaw
warsaw requested changes Aug 4, 2022
View reviewed changes

@warsaw warsaw left a comment

Copy link
Copy Markdown
Member

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.

I have a small rewording request.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot removed the awaiting change review label Aug 4, 2022
@bedevere-bot

bedevere-bot commented Aug 4, 2022

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added the awaiting changes label Aug 4, 2022
@arhadthedev
Address the Barry's review
815cb7c
@arhadthedev

arhadthedev commented Aug 4, 2022

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting change review and removed awaiting changes labels Aug 4, 2022
@bedevere-bot

bedevere-bot commented Aug 4, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot
bedevere-bot requested a review from warsaw August 4, 2022 19:25
@warsaw
Update 3.12.rst
e351af3 
Wording suggestion.
@brettcannon

brettcannon commented Aug 5, 2022

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@warsaw
Update 3.12.rst
1d157ec 
Small reword suggestion.
@warsaw

warsaw commented Aug 6, 2022

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...
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@warsaw warsaw added the 🤖 automerge label Aug 6, 2022
warsaw
warsaw approved these changes Aug 6, 2022
View reviewed changes

@warsaw warsaw left a comment

Copy link
Copy Markdown
Member

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.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting change review labels Aug 6, 2022
@miss-islington

miss-islington commented Aug 6, 2022

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@miss-islington
miss-islington merged commit 56d16e8 into python:main Aug 6, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Aug 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
@arhadthedev @iritkatriel
pythongh-93243: Make smtpd private before porting its users (pythonGH…
10dc75f 
…-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
@sobolevn sobolevn mentioned this pull request Sep 5, 2022
gh-72719: Remove deprecated asyncore and asynchat modules #96580
Merged
@arhadthedev arhadthedev mentioned this pull request Oct 6, 2022
tests still rely on the removed stdlib modules asyncore/asynchat #93243
Closed
2 of 12 tasks
@hugovk hugovk mentioned this pull request Oct 28, 2022
Remove PCbuild/lib.pyproj #98817
Closed
@vstinner vstinner mentioned this pull request Nov 8, 2022
Remove asyncore, asynchat and smtpd modules #72719
Closed
@sobolevn sobolevn mentioned this pull request Jan 21, 2024
gh-108303: Move smtpd to test.support #114368
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hugovk hugovk hugovk left review comments
@AA-Turner AA-Turner AA-Turner left review comments
@warsaw warsaw warsaw approved these changes
@vsajip vsajip Awaiting requested review from vsajip vsajip is a code owner
@brettcannon brettcannon Awaiting requested review from brettcannon

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@arhadthedev @hugovk @bedevere-bot @brettcannon @warsaw @miss-islington @AA-Turner
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.