Skip to content
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

gh-103092: Isolate winreg #103250

Merged
merged 34 commits into from
Apr 17, 2023
Merged

gh-103092: Isolate winreg #103250

merged 34 commits into from
Apr 17, 2023

Conversation

aisk
Copy link
Member

@aisk aisk commented Apr 4, 2023 •
edited by bedevere-bot
Loading

@aisk aisk requested a review from a team as a code owner April 4, 2023 16:08
@aisk aisk marked this pull request as draft April 4, 2023 16:09
@bedevere-bot bedevere-bot mentioned this pull request Apr 4, 2023
Open
25 tasks
arhadthedev
arhadthedev reviewed Apr 5, 2023
View reviewed changes
PC/winreg.c Outdated Show resolved Hide resolved
aisk and others added 4 commits April 6, 2023 15:25
@aisk aisk marked this pull request as ready for review April 7, 2023 16:50
@Eclips4
Copy link
Member

Eclips4 commented Apr 8, 2023 •
edited
Loading

Hm.. CI/CD says there is a certain amount of refleaks

@aisk
Copy link
Member Author

aisk commented Apr 8, 2023

Yes, working on it

@erlend-aasland erlend-aasland self-requested a review April 8, 2023 19:48
@erlend-aasland erlend-aasland changed the title gh-103092: isolate winreg gh-103092: Isolate winreg Apr 8, 2023
erlend-aasland
erlend-aasland reviewed Apr 8, 2023
View reviewed changes
Lib/test/test_winreg.py Show resolved Hide resolved
Misc/NEWS.d/next/Library/2023-04-08-00-48-40.gh-issue-103092.5EFts0.rst Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
aisk and others added 10 commits April 9, 2023 13:59
@aisk @erlend-aasland
Update Lib/test/test_winreg.py
71547ef 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update Misc/NEWS.d/next/Library/2023-04-08-00-48-40.gh-issue-103092.5…
1b5b873 
…EFts0.rst

Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
a4eb9c5 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
179416a 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
73f3ffe 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
dff3839 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
8623719 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk
fix gc track on PyHKEY
3f1cd90
@aisk
using Py_Type to get class instead of using defining_class and
f3da318 
using PyModule_GetState instead of _PyModule_GetState
@aisk
fix the memory leak
a2017d1
@aisk aisk requested a review from erlend-aasland April 9, 2023 12:06
erlend-aasland
erlend-aasland reviewed Apr 9, 2023
View reviewed changes
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

Choose a reason for hiding this comment

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

Thanks, I think we're soon ready for landing (and of course a thumbs up from Steve or someone else from the Windows team).

I left some more nit picks :)

Lib/test/test_winreg.py Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
Eclips4
Eclips4 reviewed Apr 9, 2023
View reviewed changes
PC/winreg.c Show resolved Hide resolved
@aisk @erlend-aasland
Update PC/winreg.c
80af80c 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
aisk and others added 6 commits April 10, 2023 17:37
@aisk @erlend-aasland
Update PC/winreg.c
488cae8 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
bbd1d6d 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
45fedf6 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
964e1dd 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @erlend-aasland
Update PC/winreg.c
16de5c6 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk @Eclips4
Update PC/winreg.c
0572407 
Co-authored-by: Kirill <80244920+Eclips4@users.noreply.github.com>
arhadthedev
arhadthedev reviewed Apr 10, 2023
View reviewed changes
PC/winreg.c Outdated Show resolved Hide resolved
erlend-aasland
erlend-aasland reviewed Apr 10, 2023
View reviewed changes
PC/winreg.c Show resolved Hide resolved
PC/winreg.c Outdated Show resolved Hide resolved
aisk and others added 2 commits April 11, 2023 23:14
@aisk @erlend-aasland
Update PC/winreg.c
682c419 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk
check result of gc new
8b1aaa4
@aisk aisk mentioned this pull request Apr 11, 2023
Merged
erlend-aasland
erlend-aasland reviewed Apr 12, 2023
View reviewed changes
PC/winreg.c Show resolved Hide resolved
PC/winreg.c Show resolved Hide resolved
aisk and others added 2 commits April 12, 2023 20:35
@aisk @erlend-aasland
Update PC/winreg.c
3ecc718 
Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com>
@aisk
remove unused PyHKEY_NumberMethods
e279c84
@aisk aisk requested a review from erlend-aasland April 12, 2023 12:41
erlend-aasland
erlend-aasland approved these changes Apr 13, 2023
View reviewed changes
PC/winreg.c Outdated Show resolved Hide resolved
@erlend-aasland erlend-aasland requested a review from zooba April 13, 2023 12:41
@erlend-aasland
Copy link
Contributor

Thanks, @aisk!

Let's wait for Steve's thumbs up before we land this.

@aisk
Copy link
Member Author

aisk commented Apr 13, 2023

Great thanks for helping and review this! @erlend-aasland

@zooba
Copy link
Member

zooba commented Apr 17, 2023

Seems okay to me, though I'm not really up on these conversions. If all the tests (esp. refleak tests) are still passing, it's presumably fine.

@erlend-aasland erlend-aasland merged commit d83faf7 into python:main Apr 17, 2023
@aisk aisk deleted the isolate-winreg branch April 18, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eclips4 Eclips4 Eclips4 left review comments

@arhadthedev arhadthedev arhadthedev left review comments

@erlend-aasland erlend-aasland erlend-aasland approved these changes

@zooba zooba Awaiting requested review from zooba

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@aisk @Eclips4 @erlend-aasland @zooba @arhadthedev @bedevere-bot