Skip to content

Conversation

@mozillazg
Copy link
Contributor

New PEP 0: python/peps#463

After merged this PR, generate_pep_pages will support both old pep-0000.txt and new pep-0000.rst.

@mozillazg mozillazg mentioned this pull request Nov 12, 2017
5 tasks
Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

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

@mozillazg sorry for my late response. Can you please rebase your branch?

Also, which one should be merged first? This one or the python/peps PR? And since we can't deploy both PRs at the same time, is there a chance that one of them cab break python.org?


data['content'] = str(pep_content)

source_link = "https://github.com/python/peps/blob/master/pep-{0}.txt".format(pep_number)
Copy link
Member

Choose a reason for hiding this comment

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

Did you move this inside get_pep_page() because you called convert_pep_page() in convert_pep0(), right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.

@mozillazg mozillazg force-pushed the convert_pep0-support-rst-format branch from 7e42c90 to 003006c Compare June 7, 2018 15:03
@mozillazg
Copy link
Contributor Author

@berkerpeksag This one should be merged first. This PR support both the old pep-0000.txt and the new pep-0000.rst. So, there is no chance that one of them can break python.org.

pep_ext = '.txt'
pep_rst_source = os.path.join(
settings.PEP_REPO_PATH, 'pep-{}.rst'.format(pep_number))
if os.path.exists(pep_rst_source):
Copy link
Member

Choose a reason for hiding this comment

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

I know you've just moved the existing code, it would be nice to use this as an opportunity to simplify the code a bit:

pep_rst_source = ...
pep_ext = '.rst' if os.path.exists(pep_rst_source) else '.txt'


pep_content = convert_pep_page(pep_number, open(pep_path).read())
pep_ext = '.txt'
pep_rst_source = os.path.join(
Copy link
Member

Choose a reason for hiding this comment

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

Style nit: Can we use hanging indentation here?

pep_rst_source = os.path.join(
    settings.PEP_REPO_PATH, 'pep-{}.rst'.format(pep_number),
)

@berkerpeksag
Copy link
Member

This PR support both the old pep-0000.txt and the new pep-0000.rst. So, there is no chance that one of them can break python.org.

Nice! :) Thank you very much.

@mozillazg mozillazg force-pushed the convert_pep0-support-rst-format branch from 003006c to 993eba8 Compare June 7, 2018 15:35
@mozillazg
Copy link
Contributor Author

Preview:

@berkerpeksag berkerpeksag merged commit 23a0332 into python:master Jun 7, 2018
@berkerpeksag
Copy link
Member

Thanks! I'll test it again in my dev environment and cherry-pick into the release branch.

berkerpeksag pushed a commit that referenced this pull request Jun 7, 2018
The new PEP 0 pull request can be found at python/peps#463.

Conflicts:

	peps/converters.py
@berkerpeksag
Copy link
Member

Cherry-picked now: 91ba765

Thanks again, @mozillazg!

Nathanator pushed a commit to Nathanator/pythondotorg that referenced this pull request Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants