bpo-29770: remove outdated PYO related info by zhangyangyu · Pull Request #590 · python/cpython (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation11 Commits8 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice cleanup! This looks good to me except I don't know anything about the following files:
- Mac/Makefile.in
- Makefile.pre.in
- Tools/buildbot/clean.bat
You may want to check these files with domain experts.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but you can re-add the PCbuild/rt.bat changes
| @@ -55,7 +55,7 @@ echo on |
|---|
| %cmd% |
| @echo off |
| echo About to run again without deleting .pyc/.pyo first: |
| echo About to run again without deleting .pyc first: |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes in rt.bat are fine, please include them. Leave Tools/buildbot/clean.bat alone, though.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the info!
@zware , I re-add rt.bat and delete related part in rmpyc.py. I can only see rmrpc.py used by rt.bat.
| npyc, npyo = deltree("../Lib") |
|---|
| print(npyc, ".pyc deleted,", npyo, ".pyo deleted") |
| npyc = deltree("../Lib") |
| print(npyc, ".pyc deleted") |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, but rmpyc.py should still delete .pyo files, just to be thorough :). If you want to reduce it to just if name.endswith(('.pyc', '.pyo')): on line 12, that would be fine with me.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to reduce it since rt.bat is modified to just mention pyc. Then still output pyo info would lead to confusion in my mind.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a bit more cleanup opportunity in PCbuild/rmpyc.py, but otherwise this LGTM.
| delete = True |
|---|
| npyc += 1 |
| elif name.endswith('.pyo'): |
| delete = True |
| npyo += 1 |
| if delete: |
| os.remove(join(root, name)) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point you're free to dispose of the delete var and just do the delete in the if branch.
zhangyangyu added a commit to zhangyangyu/cpython that referenced this pull request
zhangyangyu added a commit to zhangyangyu/cpython that referenced this pull request
zhangyangyu added a commit that referenced this pull request
zhangyangyu added a commit that referenced this pull request
Labels
Documentation in the Doc dir