-
-
Notifications
You must be signed in to change notification settings - Fork 31.2k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
GH-94851: check refcnt of immortal objects after finalization #95001
Conversation
41d0cfc
to
f66d0ce
Compare
f66d0ce
to
21ced2f
Compare
🤖 New build scheduled with the buildbot fleet by @kumaraditya303 for commit 21ced2f 🤖 If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again. |
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.
Other than one small thing, LGTM. I suspect we will rip this out once (if) PEP 683 lands. Even then this is a helpful stopgap.
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 |
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.
Thanks for adding the constant! There's a comment to add and then this should be ready to merge.
Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
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
Thanks for the reviews @ericsnowcurrently! |
Automerge-Triggered-By: GH:ericsnowcurrently