GeezerVison

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

GeezerVison is showing github.com — bpo-1635741: Adapt _multibytecodec to multi-phase initialization by erlend-aasland · Pull Request #24095 · python/cpython

Conversation

erlend-aasland commented Jan 4, 2021 •

edited by bedevere-bot

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor

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

https://bugs.python.org/issue1635741

Sorry, something went wrong.

Uh oh!

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

All reactions

Erlend E. Aasland added 7 commits

January 4, 2021 13:48


          Add empty module state

863d974


          Convert encoder and decoder types to heap type

c2c8b04


          Convert reader and writer types to heap type

2e7e452


          Convert multibytecode type to heap type

5eb463f


          Support multi-phase init

85ed337


          Add NEWS

f30a096


          Fix type names

1c9e8ba

the-knights-who-say-ni added the CLA signed label

Jan 4, 2021

bedevere-bot added the awaiting review label

Jan 4, 2021

erlend-aasland commented Jan 4, 2021

Copy link

Copy Markdown

Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

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

shihai1991 reviewed

Jan 4, 2021

View reviewed changes

shihai1991 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.

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

Sorry, something went wrong.

Uh oh!

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

All reactions

Comment thread

Misc/NEWS.d/next/Library/2021-01-04-15-05-40.bpo-1635741.EOCfZY.rst Resolved

Uh oh!

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

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

shihai1991 requested review from corona10, encukou and vstinner

January 4, 2021 16:10

Erlend E. Aasland added 2 commits

January 4, 2021 20:17


          Address review: Improve NEWS text

dc73713


          Address review: Improve naming

1b5b9fd

erlend-aasland requested a review from shihai1991

January 4, 2021 19:21

vstinner reviewed

Jan 4, 2021

View reviewed changes

Comment thread

Misc/NEWS.d/next/Library/2021-01-04-15-05-40.bpo-1635741.EOCfZY.rst Resolved

Uh oh!

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

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

Erlend E. Aasland and others added 5 commits

January 4, 2021 21:44


          Address review: Add MODULE_NAME macro

94c15cb


          Address review: Fix macro indent

15205cf


          Update Modules/cjkcodecs/multibytecodec.c

6bae5b1

Co-authored-by: Victor Stinner <[email protected]>


          Address review: undef ADD_TYPE and CREATE_TYPE macros to better show …

b1f3f3b

…their scope


          Collect AC def's and scope clinic_get_state()

e8b3021

erlend-aasland force-pushed the multibytecodec-module-state branch from 90deaf4 to e8b3021 Compare

January 4, 2021 20:55


          Address review: Improve NEWS again

883f766

erlend-aasland requested a review from vstinner

January 4, 2021 20:58

erlend-aasland commented Jan 4, 2021

Copy link

Copy Markdown

Contributor Author

Thanks for your review, @vstinner. PTAL.

All reactions

Sorry, something went wrong.

Uh oh!

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

vstinner reviewed

Jan 4, 2021

View reviewed changes

Comment thread

Modules/cjkcodecs/multibytecodec.c Resolved

Uh oh!

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

Erlend Egeberg Aasland and others added 2 commits

January 4, 2021 22:08


          Rename __methods to _multibytecodec_methods for consistency

18c1829

Co-authored-by: Victor Stinner <[email protected]>


          Fix rename

bf3da94

erlend-aasland requested a review from vstinner

January 4, 2021 21:27

vstinner merged commit 0b858cd into python:master

Jan 4, 2021

bedevere-bot removed the awaiting review label

Jan 4, 2021

vstinner commented Jan 4, 2021

Copy link

Copy Markdown

Member

Merged, thanks.

erlend-aasland reacted with hooray emoji

All reactions

🎉 1 reaction

Sorry, something went wrong.

Uh oh!

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

erlend-aasland deleted the multibytecodec-module-state branch

January 4, 2021 21:37

erlend-aasland commented Jan 4, 2021

Copy link

Copy Markdown

Contributor Author

Merged, thanks.

Great, thanks for a thorough review!

All reactions

Sorry, something went wrong.

Uh oh!

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

neonene mentioned this pull request

Jan 8, 2021

bpo-42846: Convert CJK codec extensions to multiphase init #24157

Merged

adorilson pushed a commit to adorilson/cpython that referenced this pull request

Mar 13, 2021


          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 mannequin mentioned this pull request

Sep 19, 2022

Py_Finalize() doesn't clear all Python objects at exit #44470

Closed

Reviewers

corona10

Awaiting requested review from corona10

encukou

Awaiting requested review from encukou

shihai1991

Awaiting requested review from shihai1991

vstinner

Awaiting requested review from vstinner

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.

5 participants

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.

GeezerVison

Conversation

erlend-aasland commented Jan 4, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erlend-aasland commented Jan 4, 2021

Uh oh!

shihai1991 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

erlend-aasland commented Jan 4, 2021

Uh oh!

Uh oh!

vstinner commented Jan 4, 2021

Uh oh!

erlend-aasland commented Jan 4, 2021

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

erlend-aasland commented Jan 4, 2021 •

edited by bedevere-bot

Loading

Uh oh!

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