Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Clean up PEP-505. #119

Merged
merged 3 commits into from
Oct 31, 2016
Merged

Clean up PEP-505. #119

merged 3 commits into from
Oct 31, 2016

Conversation

mehaase
Copy link
Contributor

@mehaase mehaase commented Oct 18, 2016

  • Replace emoji with C# spelling for operators.
  • Remove some unnecessary alternative spellings.
  • Add proper python code blocks.
  • Other miscellany.

@mehaase
Clean up PEP-505.
9d2518c 
* Replace emoji with C# spelling for operators.
* Remove some unnecessary alternative spellings.
* Add proper python code blocks.
* Other miscellany.
@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

Unfortunately our records indicate you have not signed a PSF contributor agreement (CLA). For legal reasons we need you to sign this before we can look at your contribution.

Once you have done everything that's needed, please reply here and someone will verify everything is in order.

@mehaase
Copy link
Contributor Author

mehaase commented Oct 18, 2016

Signed contributor agreement has been submitted.

@brettcannon
Copy link
Member

The build check failed, @mehaase, because pygments isn't installed for the PEPs. I think if you remove your code block directives -- e.g. .. code:: csharp -- that will solve the build failure.

Mariatta
Mariatta reviewed Oct 27, 2016
View reviewed changes
pep-0505.txt
@@ -302,7 +315,7 @@ from a SQL database and formats it as JSON to send to an HTTP client::

Both ``first_seen`` and ``last_seen`` are allowed to be ``null`` in the
database, and they are also allowed to be ``null`` in the JSON response. JSON
does not have a native way to represent a ``datetime``, so the server's
does not have a native way to represent a ``datetime``, so the the server's
Copy link
Member

Choose a reason for hiding this comment

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

You have an extra the here.

pep-0505.txt
@@ -504,10 +526,12 @@ Usage Of ``None`` In The Standard Library

The previous sections show some code patterns that are claimed to be "common",
but how common are they? The attached script `find-pep505.py
<https://github.com/python/peps/blob/master/pep-0505/find-pep505.py>`_ is meant to
<https://hg.python.org/peps/file/tip/pep-0505/find-pep505.py>`_ is meant to
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason for pointing to mercurial peps repo instead of github?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I didn't realize that somebody rewrote this link when I submitted it to the peps e-mail list last year. Fixed in latest commit.

pep-0505.txt
@@ -577,11 +603,11 @@ The script prints out any matches it finds. Sample::
hand, we assume that ``and`` is always incorrect for safe navigation.

The script has been tested against `test.py
<https://github.com/python/peps/blob/master/pep-0505/test.py>`_ and the Python 3.4
<https://hg.python.org/peps/file/tip/pep-0505/test.py>`_ and the Python 3.4
Copy link
Member

Choose a reason for hiding this comment

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

Use github url here?

pep-0505.txt
standard library, but it should work on any arbitrary Python 3 source code. The
complete output from running it against the standard library is attached to this
proposal as
`find-pep505.out <https://github.com/python/peps/blob/master/pep-0505/find-pep505.out>`_.
proposal as `find-pep505.out <https://hg.python.org/peps/raw-file/tip/pep-0505
Copy link
Member

Choose a reason for hiding this comment

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

here too

@mehaase
Update PEP-505
b829362 
* Fix broken build (remove code:: blocks)
* Address feedback from Mariatta
@mehaase
PEP-0505: Commmit to a specific syntax
a68b437 
I've removed all of the alternative syntax discussion and the
community poll. I expanded the reasoning for picking the spellings
that I did.
@gvanrossum gvanrossum merged commit b968fe9 into python:master Oct 31, 2016
@gvanrossum
Copy link
Member

I've merged this for you. That does not mean an endorsement. If you have further updates please create a new pull request.

@hugovk hugovk mentioned this pull request Sep 18, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mariatta Mariatta Mariatta left review comments

Assignees
No one assigned
Labels
CLA signed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@mehaase @the-knights-who-say-ni @brettcannon @gvanrossum @Mariatta