◐ Shell
reader mode source ↗
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

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

Copy link
Copy Markdown
Member

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

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

Hide comment

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
Member Author

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 owners June 10, 2020 11:25
@pablogsal pablogsal changed the title Remove the old parser Jun 10, 2020
@pablogsal pablogsal force-pushed the remove_old_parser branch from 62192dd to 3dc1827 Compare June 10, 2020 13:09
lysnikolaou
lysnikolaou approved these changes Jun 10, 2020
View reviewed changes

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

Hide comment

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 pablogsal force-pushed the remove_old_parser branch 2 times, most recently from a4055e4 to 738a7ae Compare June 10, 2020 23:58
@pablogsal

pablogsal commented Jun 11, 2020

Copy link
Copy Markdown
Member Author

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.

@pablogsal pablogsal force-pushed the remove_old_parser branch 2 times, most recently from e11e688 to 1ad8ff2 Compare June 11, 2020 15:22
@pablogsal

pablogsal commented Jun 11, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

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

pablogsal added 3 commits June 11, 2020 17:10
@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
@pablogsal pablogsal force-pushed the remove_old_parser branch from 1ad8ff2 to 635a9e2 Compare June 11, 2020 16:10
23 hidden items Load more…
gvanrossum
gvanrossum approved these changes Jun 11, 2020
View reviewed changes

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

Hide comment

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
Member Author

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:master Jun 11, 2020
@pablogsal pablogsal deleted the remove_old_parser branch June 11, 2020 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lysnikolaou lysnikolaou lysnikolaou approved these changes

@gvanrossum gvanrossum gvanrossum approved these changes

@gpshead gpshead Awaiting requested review from gpshead

+1 more reviewer

@remilapeyre remilapeyre remilapeyre 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