Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

bpo-29770: remove outdated PYO related info#590

Merged
zhangyangyu merged 8 commits into
python:masterpython/cpython:masterfrom
zhangyangyu:bpo-29770zhangyangyu/cpython:bpo-29770Copy head branch name to clipboard
Mar 11, 2017
Merged

bpo-29770: remove outdated PYO related info#590
zhangyangyu merged 8 commits into
python:masterpython/cpython:masterfrom
zhangyangyu:bpo-29770zhangyangyu/cpython:bpo-29770Copy head branch name to clipboard

Conversation

@zhangyangyu

Copy link
Copy Markdown
Member

No description provided.

@mention-bot

Copy link
Copy Markdown

@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 berkerpeksag left a comment

Copy link
Copy Markdown
Member

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.

@zhangyangyu zhangyangyu added the docs Documentation in the Doc dir label Mar 10, 2017
@zhangyangyu

zhangyangyu commented Mar 10, 2017

Copy link
Copy Markdown
Member Author

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 zware left a comment

Copy link
Copy Markdown
Member

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

Comment thread PCbuild/rt.bat
@echo off

echo About to run again without deleting .pyc/.pyo first:
echo About to run again without deleting .pyc first:

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for the info!

@zhangyangyu

Copy link
Copy Markdown
Member Author

@zware , I re-add rt.bat and delete related part in rmpyc.py. I can only see rmrpc.py used by rt.bat.

Comment thread PCbuild/rmpyc.py
npyc, npyo = deltree("../Lib")
print(npyc, ".pyc deleted,", npyo, ".pyo deleted")
npyc = deltree("../Lib")
print(npyc, ".pyc deleted")

Copy link
Copy Markdown
Member

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.

Copy link
Copy Markdown
Member Author

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.

@zware zware left a comment

Copy link
Copy Markdown
Member

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.

Comment thread PCbuild/rmpyc.py
npyo += 1

if delete:
os.remove(join(root, name))

Copy link
Copy Markdown
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.