Skip to content

bpo-40939: Remove the old parser - #20768

Merged
pablogsal merged 3 commits into
python:masterfrom
pablogsal:remove_old_parser
Jun 11, 2020
Merged

bpo-40939: Remove the old parser#20768
pablogsal merged 3 commits into
python:masterfrom
pablogsal:remove_old_parser

Conversation

@pablogsal

@pablogsalpablogsal commented Jun 9, 2020•
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@pablogsalpablogsal mentioned this pull request Jun 9, 2020
Closed
@pablogsal
pablogsalforce-pushed the remove_old_parser branch 6 times, most recently from c1a2097 to 27a0c11CompareJune 10, 2020 00:29
remilapeyre
remilapeyre reviewed Jun 10, 2020
View reviewed changes

@remilapeyreremilapeyre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hi, there is also a note in Doc/using/cmdline.rst that will need to be removed.

@pablogsal

pablogsal commented Jun 10, 2020

Copy link
Copy Markdown
MemberAuthor

Hi, there is also a note in Doc/using/cmdline.rst that will need to be removed.

Thanks for the catch!

@pablogsal
pablogsal marked this pull request as ready for review June 10, 2020 11:25
@pablogsal
pablogsal requested review from a team and gpshead as code ownersJune 10, 2020 11:25
@pablogsalpablogsal changed the title Remove the old parserbpo-40939: Remove the old parserJun 10, 2020
lysnikolaou
lysnikolaou approved these changes Jun 10, 2020
View reviewed changes

@lysnikolaoulysnikolaou 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.

This LGTM! Amazing work, I couldn't find a single miss! 🚀

Also, I guess this is what we've been really working for in the last months. Feels good!

@pablogsal
pablogsalforce-pushed the remove_old_parser branch 2 times, most recently from a4055e4 to 738a7aeCompareJune 10, 2020 23:58
@pablogsal

pablogsal commented Jun 11, 2020

Copy link
Copy Markdown
MemberAuthor

I plan to land this tomorrow as this PR keeps getting merge conflicts from everywhere and it will be easier to fix small things in future PRs if needed.

gvanrossum
gvanrossum reviewed Jun 11, 2020
View reviewed changes
Comment threadLib/test/test_peg_parser.py Outdated

@gvanrossumgvanrossumJun 11, 2020

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.

why is this file deleted?

@pablogsalpablogsalJun 11, 2020

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

This file is comparing the ast of the old parser to the last of the new parser, so it serves no purpose anymore. We have been adding all negative cases (testing extensions) into other files so we won't lose anything

Comment threadLib/test/test_string_literals.py Outdated
Comment threadMakefile.pre.in Outdated
Comment threadMakefile.pre.in Outdated
Comment threadModules/Setup Outdated
@pablogsal
pablogsalforce-pushed the remove_old_parser branch 2 times, most recently from e11e688 to 1ad8ff2CompareJune 11, 2020 15:22
@pablogsal

pablogsal commented Jun 11, 2020•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

I have rebased again to solve more merge conflicts. Could you review again?

@pablogsal
Remove the old parser
51d73f8
Update cmdline.rst
📜🤖 Added by blurb_it.
Clean some tests and bring back some deleted cases
@pablogsal
Restore tests in test_string_literals
1118380
@pablogsal
Rename parser files
635a9e2
gvanrossum
gvanrossum approved these changes Jun 11, 2020
View reviewed changes

@gvanrossumgvanrossum 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.

I wonder if we could backport the file renames? (Not the moves from Parser/pegen/* to Parser/*, but the rename of parse.c to parser.c and parse_string.c to string_parser.c.)

@pablogsal

pablogsal commented Jun 11, 2020

Copy link
Copy Markdown
MemberAuthor

I wonder if we could backport the file renames? (Not the moves from Parser/pegen/* to Parser/*, but the rename of parse.c to parser.c and parse_string.c to string_parser.c.)

👍 I will try to make a PR later today with that

@gvanrossum

gvanrossum commented Jun 11, 2020 via email

Copy link
Copy Markdown
Member

@pablogsal
pablogsal merged commit 1ed83ad into python:masterJun 11, 2020
@pablogsal
pablogsal deleted the remove_old_parser branch June 11, 2020 16:30
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lysnikolaoulysnikolaoulysnikolaou approved these changes
@gvanrossumgvanrossumgvanrossum approved these changes
@gpsheadgpsheadAwaiting requested review from gpshead
+1 more reviewer
@remilapeyreremilapeyreremilapeyre left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@pablogsal@gvanrossum@lysnikolaou@remilapeyre@the-knights-who-say-ni@bedevere-bot