Skip to content

bpo-46541: Remove usage of _Py_IDENTIFIER from multibytecodec - #31475

Merged
corona10 merged 3 commits into
python:mainfrom
corona10:bpo-46541-multibytecodec
Mar 1, 2022
Merged

bpo-46541: Remove usage of _Py_IDENTIFIER from multibytecodec#31475
corona10 merged 3 commits into
python:mainfrom
corona10:bpo-46541-multibytecodec

Conversation

@corona10

@corona10corona10 commented Feb 21, 2022

Copy link
Copy Markdown
Member

Comment threadModules/cjkcodecs/multibytecodec.c Outdated
@terryjreedy

terryjreedy commented Feb 21, 2022

Copy link
Copy Markdown
Member

In the issue number, 65 was transposed as 56. Fixed.

@terryjreedyterryjreedy changed the title bpo-45641: Remove usage of _Py_IDENTIFIER from multibytecodecbpo-46541: Remove usage of _Py_IDENTIFIER from multibytecodecFeb 21, 2022

@kumaraditya303kumaraditya303 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

Comment threadModules/cjkcodecs/multibytecodec.c Outdated
@bedevere-bot

Copy link
Copy Markdown

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

@corona10

Copy link
Copy Markdown
MemberAuthor

I have made the requested changes; please review again

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@tiran: please review the changes made to this pull request.

@corona10

Copy link
Copy Markdown
MemberAuthor

After some discussion, I decided to merge this PR anyway.
I will accept the other approach if someone wants :) I hope that this approach would help to reduce the usage of private APIs
cc @ericsnowcurrently, @erlend-aasland, @tiran, @vstinner (alphabetical order :)

@corona10
corona10 merged commit 0cc6364 into python:mainMar 1, 2022
@corona10
corona10 deleted the bpo-46541-multibytecodec branch March 1, 2022 14:35
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

@corona10@terryjreedy@bedevere-bot@tiran@erlend-aasland@kumaraditya303@the-knights-who-say-ni