Skip to content

bpo-42299: Remove formatter module #23476

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.

Sign up for GitHub

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

Jump to bottom
Merged
corona10 merged 5 commits into from
Nov 25, 2020
Merged

bpo-42299: Remove formatter module #23476

corona10 merged 5 commits into from
Nov 25, 2020

Conversation

@corona10
Copy link
Member

@corona10 corona10 commented Nov 23, 2020 •
edited by bedevere-bot
Loading

@corona10 corona10 requested a review from terryjreedy November 23, 2020 14:46
vstinner
vstinner reviewed Nov 24, 2020
View reviewed changes
vstinner
vstinner reviewed Nov 24, 2020
View reviewed changes
@corona10 corona10 requested a review from vstinner November 24, 2020 14:21
corona10
corona10 commented Nov 24, 2020
View reviewed changes
Copy link
Member Author

@corona10 corona10 left a comment

Choose a reason for hiding this comment

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

@vstinner Can you please take a look?

vstinner
vstinner reviewed Nov 24, 2020
View reviewed changes
vstinner
vstinner approved these changes Nov 24, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. I'm fine with removing the module. I just left minor remarks on the documentation, you're free to ignore them.

terryjreedy
terryjreedy reviewed Nov 24, 2020
View reviewed changes
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

LGTM. I simply don't know if anything else is needed.

terryjreedy
terryjreedy requested changes Nov 24, 2020
View reviewed changes
Copy link
Member

@terryjreedy terryjreedy left a comment

Choose a reason for hiding this comment

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

See suggested change, which applies to news also.

@bedevere-bot
Copy link

bedevere-bot commented Nov 24, 2020

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

@corona10 @terryjreedy
Update Doc/whatsnew/3.10.rst
60ec82a 
Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>
vstinner
vstinner approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @terryjreedy for the way better documentation than what I wrote :-) It's good to write down the full rationale, otherwise people get angry when they see that we remove modules on purpose.

@corona10
Copy link
Member Author

corona10 commented Nov 25, 2020

@vstinner @terryjreedy
Thanks Victor,
I will merge this PR after CI is passed ;)

@corona10 corona10 merged commit be319c0 into python:master Nov 25, 2020
@corona10 corona10 deleted the bpo-42299 branch November 25, 2020 13:17
@terryjreedy
Copy link
Member

terryjreedy commented Nov 25, 2020

Victor, I was inspired to add a 'solution' by your sentence from issue 39352: "I'm no longer sure that it's a good idea to remove a module from the stdlib without providing a solution for the few people relying on it." In this case, I am convinced that copying is a better solution than putting the module on PyPI.

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@corona10 @adorilson
bpo-42299: Remove formatter module (pythonGH-23476)
0e35556
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@terryjreedy terryjreedy terryjreedy requested changes

@vstinner vstinner vstinner approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@corona10 @bedevere-bot @terryjreedy @vstinner @the-knights-who-say-ni