Skip to content

bpo-42327: Add PyModule_Add(). #23240

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 12 commits into from
Closed

bpo-42327: Add PyModule_Add(). #23240

wants to merge 12 commits into from

Conversation

serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented Nov 11, 2020 •
edited by bedevere-bot
Loading

@serhiy-storchaka serhiy-storchaka changed the title Add PyModule_Add(). bpo-42327: Add PyModule_Add(). Nov 11, 2020
@serhiy-storchaka serhiy-storchaka marked this pull request as ready for review November 14, 2020 17:47
@pablogsal pablogsal added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 14, 2020
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @pablogsal for commit 17279ae 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 14, 2020
tiran
tiran requested changes Nov 14, 2020
View reviewed changes
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

Could you please move your changes of _ssl.c to another PR? This PR will cause major conflicts with two of my PRs.

I also like to get your opinion on https://discuss.python.org/t/define-module-constants-in-pymoduledef/5749 . I have suggested a radically different approach for module constant initialization that is even more convenient than PyModule_Add().

@bedevere-bot
Copy link

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

@serhiy-storchaka
Copy link
Member Author

I will most likely break this PR into several parts and merge them separately. It grew more than I expected because for each modified module I fixed all potential leaks, and some module expose many constants.

@serhiy-storchaka serhiy-storchaka mentioned this pull request Nov 21, 2020
Merged
@serhiy-storchaka
Copy link
Member Author

Created #23443 which does not include large changes.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Dec 22, 2020
@rhettinger rhettinger removed their request for review May 3, 2022 06:14
@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Jul 31, 2022
@serhiy-storchaka serhiy-storchaka mentioned this pull request Nov 15, 2022
Closed
@serhiy-storchaka
Copy link
Member Author

Split and merged as a number of smaller PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiran tiran tiran requested changes

@abalkin abalkin Awaiting requested review from abalkin abalkin is a code owner

@pablogsal pablogsal Awaiting requested review from pablogsal

@pganssle pganssle Awaiting requested review from pganssle pganssle is a code owner

Assignees
No one assigned
Labels
awaiting changes DO-NOT-MERGE
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@serhiy-storchaka @bedevere-bot @tiran @pablogsal @the-knights-who-say-ni @ezio-melotti