Skip to content

/ cpython

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

bpo-39366: Remove xpath() and xgtitle() methods of NNTP. #18035

Open
wants to merge 4 commits into
base: master
from
Open

bpo-39366: Remove xpath() and xgtitle() methods of NNTP. #18035

wants to merge 4 commits into from
+9 −66

Conversation

@corona10
Copy link
Member

corona10 commented Jan 17, 2020 •
edited

https://bugs.python.org/issue39366
@corona10 corona10 requested a review from vstinner Jan 17, 2020
@corona10 corona10 changed the title bpo-39336: Remove xpath() and xgtitle() methods of NNTP. bpo-39366: Remove xpath() and xgtitle() methods of NNTP. Jan 17, 2020
@corona10
bpo-39366: Remove xpath() and xgtitle() methods of NNTP.
Loading status checks…
3c9748d
@corona10 corona10 force-pushed the corona10:bpo-39336 branch from b5d47c4 to 3c9748d Jan 17, 2020
@vstinner
vstinner reviewed Jan 17, 2020
View changes
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
@corona10
bpo-39366: Doc update
Loading status checks…
e8868ed
@corona10 corona10 force-pushed the corona10:bpo-39336 branch from 075d0f2 to e8868ed Jan 17, 2020
@vstinner
vstinner approved these changes Jan 17, 2020
View changes
Copy link
Member

vstinner left a comment

LGTM. But I would prefer to have a second review, since I'm not used to nntplib.

@serhiy-storchaka @vadmium @berkerpeksag @pitrou: Can one of you please review this PR as well? Well, IMO the PR itself is fine, the question is more if you are ok to drop these two deprecated methods.
Doc/whatsnew/3.9.rst Show resolved Hide resolved
@corona10
bpo-39366: Update whatsnew 3.9
Loading status checks…
665cc7d
@berkerpeksag
berkerpeksag requested changes Jan 19, 2020
View changes
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Doc/whatsnew/3.9.rst Outdated Show resolved Hide resolved
Misc/NEWS.d/next/Library/2020-01-17-18-14-51.bpo-39366.Cv3NQS.rst Outdated Show resolved Hide resolved
Lib/nntplib.py Show resolved Hide resolved
@bedevere-bot

This comment has been minimized.

Sign in to view
Copy link

bedevere-bot commented Jan 19, 2020

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.
@corona10
bpo-39366: Apply codereview
Loading status checks…
0b80584
@corona10
corona10 reviewed Jan 20, 2020
View changes
Copy link
Member Author

corona10 left a comment •
edited

@berkerpeksag

Thanks for the review!
I have made the requested changes; please review again
@corona10 corona10 requested a review from berkerpeksag Jan 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vstinner vstinner

@berkerpeksag berkerpeksag

Assignees
No one assigned
Labels
CLA signed awaiting changes
Projects
None yet
Milestone
No milestone
5 participants
@corona10 @bedevere-bot @berkerpeksag @vstinner @the-knights-who-say-ni
You can’t perform that action at this time.