Skip to content

Conversation

@vstinner
Copy link
Member

@vstinnervstinner commented Apr 10, 2024

@vstinner
Copy link
MemberAuthor

This PR is based on PR gh-117626.

@vstinner
Copy link
MemberAuthor

PR rebased on top of merged #117626.

@vstinnervstinner marked this pull request as ready for review April 11, 2024 10:17
@erlend-aaslanderlend-aasland changed the title [WIP] gh-113317: Add ParseArgsCodeGen classgh-113317: Add ParseArgsCodeGen classApr 11, 2024
@erlend-aasland
Copy link
Contributor

The previous PR used the Codegen spelling. This PR uses CodeGen. Consider using only one variant :)

@erlend-aasland
Copy link
Contributor

This is nice!

Should we consider adding this as a separate module? Or would libclinic.codegen be a better home for it?

@vstinner
Copy link
MemberAuthor

Should we consider adding this as a separate module?

CLanguage uses ParseArgsCodeGen which uses CLanguage... There is an inter-dependency. It's more convenient to have both classes in the same file.

@erlend-aasland
Copy link
Contributor

Yes, there are inter-dependencies everywhere :(

@vstinner
Copy link
MemberAuthor

The previous PR used the Codegen spelling. This PR uses CodeGen. Consider using only one variant :)

I updated this PR to rename Codegen to CodeGen.

@vstinner
Copy link
MemberAuthor

Should we consider adding this as a separate module?

I broke the inter-dependency between CLanguage and ParseArgsCodeGen, and I moved declare_parser() and ParseArgsCodeGen to a new libclinic.parse_args module.

@erlend-aasland
Copy link
Contributor

I broke the inter-dependency between CLanguage and ParseArgsCodeGen, and I moved declare_parser() and ParseArgsCodeGen to a new libclinic.parse_args module.

Nice, so now we can make the parser templates globals of the libclinic.parse_args module, instead of class members.

@vstinner
Copy link
MemberAuthor

Nice, so now we can make the parser templates globals of the libclinic.parse_args module, instead of class members.

Done.

@vstinnervstinner enabled auto-merge (squash) April 11, 2024 13:48
@vstinnervstinner merged commit 671cb22 into python:mainApr 11, 2024
@vstinnervstinner deleted the clinic_parse_args branch April 11, 2024 13:49
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@vstinner@erlend-aasland