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.

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

bpo-45019: Cleanup module freezing and deepfreeze #29772

Merged
merged 2 commits into from Nov 26, 2021

Conversation

@kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented Nov 25, 2021

https://bugs.python.org/issue45019

@kumaraditya303 kumaraditya303 marked this pull request as ready for review Nov 25, 2021
@kumaraditya303 kumaraditya303 requested a review from as a code owner Nov 25, 2021
@kumaraditya303
Copy link
Contributor Author

@kumaraditya303 kumaraditya303 commented Nov 25, 2021

Loading

Copy link
Member

@gvanrossum gvanrossum left a comment

Without looking carefully yet, I think I like your cleanup of freeze_modules.py, but not the rename of the deepfreeze directory. Can you undo the latter and just focus on improving the script?

Loading

@kumaraditya303
Copy link
Contributor Author

@kumaraditya303 kumaraditya303 commented Nov 25, 2021

Without looking carefully yet, I think I like your cleanup of freeze_modules.py, but not the rename of the deepfreeze directory. Can you undo the latter and just focus on improving the script?

EDIT: @gvanrossum done

Loading

@kumaraditya303 kumaraditya303 requested review from gvanrossum and removed request for Nov 25, 2021
Copy link
Member

@gvanrossum gvanrossum left a comment

This is great. I have a few nits.

I also wonder if there aren't more improvements we can make, e.g. use relative paths throughout the code, rather than computing absolute paths and then making them relative for the generated code again.

Oh, and it would be nice to be able to freeze some submodules in a package but not all of them -- e.g. I'd like to freeze encodings/{__init__,utf_8,aliases}.py but not the remaining 100 submodules in that package.

Loading

Tools/scripts/freeze_modules.py Outdated Show resolved Hide resolved
Loading
Tools/scripts/freeze_modules.py Outdated Show resolved Hide resolved
Loading
@kumaraditya303
Copy link
Contributor Author

@kumaraditya303 kumaraditya303 commented Nov 26, 2021

Freeze encoding here #29788

Loading

@kumaraditya303 kumaraditya303 requested a review from gvanrossum Nov 26, 2021
Copy link
Member

@gvanrossum gvanrossum left a comment

Looks good for this phase! I'll merge this now. Then you can work on e.g. changes groups of functions to classes and partially freezing encodings.

Loading

@gvanrossum gvanrossum merged commit b0b10e1 into python:main Nov 26, 2021
11 checks passed
Loading
@bedevere-bot
Copy link

@bedevere-bot bedevere-bot commented Nov 26, 2021

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

Loading

@kumaraditya303
Copy link
Contributor Author

@kumaraditya303 kumaraditya303 commented Nov 26, 2021

Thanks @gvanrossum This was my first contribution to cpython, earlier was just a typo fix, glad that you merged it :).

Loading

@kumaraditya303 kumaraditya303 deleted the freeze branch Nov 26, 2021
@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Nov 26, 2021

Thanks Kumar for cleaning up our mess! :-) I am looking forward to seeing more from you.

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants