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.

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

[generate-ceval-switch] GH-98831: "Generate" the interpreter (GH-98830) #65

Closed
wants to merge 1 commit into from
Closed

Conversation

qexat
Copy link

@qexat qexat commented Oct 31, 2022

I noticed that some files in Tools/cases_generator/ have their code in the main() function, but others don't. This PR proposes to move lexer.py, parser.py, plexer.py and sparser.py main code to a main() function for consistency.

@qexat
Copy link
Author

qexat commented Oct 31, 2022

Sorry if the commit message does not fit Python's repo standards, I was not sure about what to put

@qexat qexat changed the title Moved stuff to a main() function [generate-ceval-switch] GH-98831: "Generate" the interpreter (GH-98830) Oct 31, 2022
@gvanrossum
Copy link
Owner

Thanks for the suggestion, but I'll leave this unmerged. There's a reason there are only two files that have a main() function -- those are the only ones that are meant as scripts (extract_cases.py and generate_cases.py). The others just have a little bit of hacky test code.

Also, note that it's uncommon to submit PRs to branches in my fork -- this is not our workflow (it may be different in some other projects). My branches are short-lived and not normally used for collaboration; if people have suggestions they tend to make them using GitHub's review UI against the PR (in this case, pythonGH-98830).

@gvanrossum gvanrossum closed this Nov 1, 2022
@qexat
Copy link
Author

qexat commented Nov 1, 2022

Understood. Sorry, I'm not very familiar with the workflow, next time I'll try to follow that.

@qexat qexat deleted the generate-ceval-switch branch November 1, 2022 03:49
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