Uh oh!
There was an error while loading. Please reload this page.
GH-90699: Remove remaining _Py_IDENTIFIER stdlib usage - #99067
Conversation
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot
commented
Nov 3, 2022
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 |
ericsnowcurrently
left a comment
There was a problem hiding this comment.
LGTM
(Thanks for the changes.)
ericsnowcurrently
left a comment
There was a problem hiding this comment.
One small thing and then LGTM.
Also, were you planning on removing _Py_IDENTIFIER() from Programs/_testembed.c? (Presumably we'd drop test_unicode_id_init().) Then again, that should probably be part of the issue/PR that actually eliminates _Py_IDENTIFIER().
Uh oh!
There was an error while loading. Please reload this page.
bedevere-bot
commented
Nov 7, 2022
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 |
kumaraditya303
commented
Nov 7, 2022
Yeah, will remove that in PR which removes |
_Py_IDENTIFIER usage_Py_IDENTIFIER stdlib usagevstinner
commented
Nov 7, 2022
Refleak buildbots fail since this commit. Example: Example: cc @pablogsal |
Please @ericsnowcurrently@kumaraditya303 take a look, otherwise we will need to revert if is not fixed in 24h per our buildbot policy. |
ericsnowcurrently
commented
Nov 8, 2022
If @kumaraditya303 doesn't get to it first, I'll tackle this first thing tomorrow morning (~15 hours from now). |
kumaraditya303
commented
Nov 8, 2022
#99236 fixes the refleak. |
vstinner
commented
Nov 8, 2022
The PR got merged, I confirm that it does fix the issue: |
ericsnowcurrently
commented
Nov 8, 2022
Thanks, @kumaraditya303! |
Automerge-Triggered-By: GH:ericsnowcurrently