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. |
berkerpeksag
left a comment
There was a problem hiding this comment.
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. :-) |
zware
left a comment
There was a problem hiding this comment.
LGTM, but you can re-add the PCbuild/rt.bat changes
| @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.
These changes in rt.bat are fine, please include them. Leave Tools/buildbot/clean.bat alone, though.
There was a problem hiding this comment.
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.
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.
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.
zware
left a comment
There was a problem hiding this comment.
There's a bit more cleanup opportunity in PCbuild/rmpyc.py, but otherwise this LGTM.
| npyo += 1 | ||
|
|
||
| if delete: | ||
| os.remove(join(root, name)) |
There was a problem hiding this comment.
At this point you're free to dispose of the delete var and just do the delete in the if branch.
No description provided.