bpo-40417: Fix deprecation warning in PyImport_ReloadModule #19750
Conversation
Robmaister
requested review from
brettcannon,
encukou,
ericsnowcurrently,
ncoghlan and
warsaw
as code owners
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
After a little bit of confusion, I was able to figure out what was going on with an error message I got when trying to set some of my info in BPO. Github account linked, CLA was signed about an hour ago so everything should be good in about a day 😄 |
There was a problem hiding this comment.
Mostly LGTM. Renaming the variable to "importlib" would be better for readers. I'll approve the PR but please consider making that change anyway. 😄
I wouldn't worry about adding a test. We don't really have a pattern (of which I'm aware at least) of verifying that the C-API does not emit deprecation warnings.
|
@Robmaister mind adding a news entry? If you want to use a web form you can follow the "Details" link for the failing bedevere/news status check. |
Misc/NEWS.d/next/Core and Builtins/2020-05-01-19-04-52.bpo-40417.Sti2lJ.rst
Outdated
Show resolved
Hide resolved
|
@Robmaister I made sure to thank you in the news entry. Once CI passes this should get auto-merged. |
|
@Robmaister: Status check is done, and it's a success ✅ . |
|
Thanks @Robmaister for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
Thanks @Robmaister for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8. |
|
Sorry, @Robmaister, I could not cleanly backport this to |
|
Sorry @Robmaister, I had trouble checking out the |
…thonGH-19750) I can add another commit with the new test case I wrote to verify that the warning was being printed before my change, stopped printing after my change, and that the function does not return null after my change. Automerge-Triggered-By: @brettcannon. (cherry picked from commit f40bd46) Co-authored-by: Robert Rouhani <robert.rouhani@gmail.com>
|
GH-19934 is a backport of this pull request to the 3.8 branch. |
…thonGH-19750) I can add another commit with the new test case I wrote to verify that the warning was being printed before my change, stopped printing after my change, and that the function does not return null after my change. Automerge-Triggered-By: @brettcannon. (cherry picked from commit f40bd46) Co-authored-by: Robert Rouhani <robert.rouhani@gmail.com>
|
GH-19935 is a backport of this pull request to the 3.7 branch. |
…-19750) (GH-19934) Automerge-Triggered-By: @brettcannon. (cherry picked from commit f40bd46) Co-authored-by: Robert Rouhani <robert.rouhani@gmail.com>
…-19750) (GH-19935) Use importlib instead of imp. Automerge-Triggered-By: @brettcannon. (cherry picked from commit f40bd46) Co-authored-by: Robert Rouhani robert.rouhani@gmail.com
I can add another commit with the new test case I wrote to verify that the warning was being printed before my change, stopped printing after my change, and that the function does not return null after my change.
https://bugs.python.org/issue40417
https://bugs.python.org/issue40417
Automerge-Triggered-By: @brettcannon