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-29770: remove outdated PYO related info #590
Conversation
@zhangyangyu, thanks for your PR! By analyzing the history of the files in this pull request, we identified @zware, @zooba, @birkenfeld, @Yhg1s and @ncoghlan to be potential reviewers. |
There was a problem hiding this comment.
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.
Thanks @berkerpeksag ! I read the original issue http://bugs.python.org/issue23731 and decide to leave all the build related files alone. So it's now a barely documentation issue. :-) |
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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") |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
PCbuild/rmpyc.py
Outdated
delete = True | ||
npyc += 1 | ||
elif name.endswith('.pyo'): | ||
delete = True | ||
npyo += 1 | ||
|
||
if delete: | ||
os.remove(join(root, name)) |
There was a problem hiding this comment.
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.
No description provided.