Skip to content

bpo-40939: Remove PEG parser easter egg #20802

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

Merged
merged 2 commits into from
Jun 11, 2020

Conversation

lysnikolaou
Copy link
Member

@lysnikolaou lysnikolaou commented Jun 11, 2020

@lysnikolaou
Copy link
Member Author

Closing and reopening for the CI.

@lysnikolaou lysnikolaou reopened this Jun 11, 2020
@lysnikolaou lysnikolaou changed the title bpo-40939: Rename PEG parser easter egg to avoid name clashes bpo-40939: Remove PEG parser easter egg Jun 11, 2020
@lysnikolaou lysnikolaou requested a review from pablogsal June 11, 2020 14:50
@lysnikolaou
Copy link
Member Author

@pablogsal I don't know if the commit message goes into the squash commit message, but here it's currently wrong, since it mentions renaming and not removing. Should I change it or will you just take the PR title as the commit message of the squash commit?

@pablogsal
Copy link
Member

Should I change it or will you just take the PR title as the commit message of the squash commit?

I will customize it when I merge :)

@gvanrossum gvanrossum merged commit bcd7dee into python:master Jun 11, 2020
@miss-islington
Copy link
Contributor

Thanks @lysnikolaou for the PR, and @gvanrossum for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@bedevere-bot
Copy link

@gvanrossum: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

Sorry @lysnikolaou and @gvanrossum, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker bcd7deed9118e365c1225de2a2e1a81bf988c6ab 3.9

@gvanrossum
Copy link
Member

Honestly I wouldn't back port this. In 3.9 this does serve a purpose.

@pablogsal pablogsal removed the needs backport to 3.9 only security fixes label Jun 11, 2020
@pablogsal
Copy link
Member

pablogsal commented Jun 11, 2020

Honestly I wouldn't back port this. In 3.9 this does serve a purpose.

A PR is still needed to fix the buildbot errors.

@gvanrossum
Copy link
Member

Okay, can you do it? I don't even know what the buildbot error is.

@pablogsal
Copy link
Member

Okay, can you do it? I don't even know what the buildbot error is.

Yep, I will take care of it don't worry

@lysnikolaou
Copy link
Member Author

lysnikolaou commented Jun 11, 2020

Yep, I will take care of it don't worry

Already got something ready. I'll push soon.

arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
It no longer serves a purpose (there's only one parser) and having "new" in any name will eventually look odd. Also, it impinges on a potential sub-namespace, `__new_...__`.
@laloch
Copy link

laloch commented Aug 30, 2020

There seem to still be some leftovers present:

$ docker run -i -t python:3.9-rc /usr/local/bin/python
Python 3.9.0rc1 (default, Aug 12 2020, 18:31:30) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import keyword
>>> keyword.iskeyword("__peg_parser__")
True
>>> assert "__peg_parser__" not in keyword.kwlist
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AssertionError

@gvanrossum
Copy link
Member

The removal applies to 3.10.

@laloch
Copy link

laloch commented Aug 30, 2020

Oh, I see! I should have read the comments I guess. Sorry for the noise.
I would like the change to get backported, bacause it prevents Xonsh from building its parser table right now, but never mind, we'll adjust. Thanks!

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

Successfully merging this pull request may close these issues.

7 participants