Skip to content
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

bpo-1635741: Adapt _multibytecodec to multi-phase initialization #24095

Merged
merged 17 commits into from
Jan 4, 2021
Merged

bpo-1635741: Adapt _multibytecodec to multi-phase initialization #24095

merged 17 commits into from
Jan 4, 2021

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jan 4, 2021 •
edited by bedevere-bot
Loading

  • Establish module state
  • Convert types to heap types and add them to module state
  • Add multi-phase init support

https://bugs.python.org/issue1635741

@erlend-aasland
Copy link
Contributor Author

@corona10 or @shihai1991, would one of you mind reviewing this PR?

shihai1991
shihai1991 reviewed Jan 4, 2021
View reviewed changes
Copy link
Member

@shihai1991 shihai1991 left a comment

Choose a reason for hiding this comment

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

LGTM. just nitpicks in here. And no refleaks when I run ./python -m test test_multibytecodec -R 3:3 -vv.

Misc/NEWS.d/next/Library/2021-01-04-15-05-40.bpo-1635741.EOCfZY.rst Outdated Show resolved Hide resolved
Modules/cjkcodecs/multibytecodec.c Outdated Show resolved Hide resolved
vstinner
vstinner reviewed Jan 4, 2021
View reviewed changes
Misc/NEWS.d/next/Library/2021-01-04-15-05-40.bpo-1635741.EOCfZY.rst Outdated Show resolved Hide resolved
Modules/cjkcodecs/multibytecodec.c Outdated Show resolved Hide resolved
Modules/cjkcodecs/multibytecodec.c Outdated Show resolved Hide resolved
Modules/cjkcodecs/multibytecodec.c Outdated Show resolved Hide resolved
Modules/cjkcodecs/multibytecodec.c Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor Author

Thanks for your review, @vstinner. PTAL.

Erlend Egeberg Aasland and others added 2 commits January 4, 2021 22:08
@vstinner
Rename __methods to _multibytecodec_methods for consistency
18c1829 
Co-authored-by: Victor Stinner <vstinner@python.org>
Fix rename
bf3da94
@vstinner vstinner merged commit 0b858cd into python:master Jan 4, 2021
@vstinner
Copy link
Member

vstinner commented Jan 4, 2021

Merged, thanks.

@erlend-aasland erlend-aasland deleted the multibytecodec-module-state branch January 4, 2021 21:37
@erlend-aasland
Copy link
Contributor Author

Merged, thanks.

Great, thanks for a thorough review!

@neonene neonene mentioned this pull request Jan 8, 2021
Merged
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@adorilson
bpo-1635741: Convert _multibytecodec to multi-phase init (pythonGH-24095
42e5f6b 
)

Convert the _multibytecodec extension module (CJK codecs) to multi-phase
initialization (PEP 489).
@kylotan kylotan mannequin mentioned this pull request Sep 19, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 Awaiting requested review from corona10

@encukou encukou Awaiting requested review from encukou

@shihai1991 shihai1991 Awaiting requested review from shihai1991

@vstinner vstinner Awaiting requested review from vstinner

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
@erlend-aasland @vstinner @shihai1991 @the-knights-who-say-ni @bedevere-bot