10000 gh-114258: Refactor Argument Clinic function name parser by erlend-aasland · Pull Request #114930 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-114258: Refactor Argument Clinic function name parser #114930

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Feb 15, 2024
Merged

gh-114258: Refactor Argument Clinic function name parser #114930

merged 12 commits into from
Feb 15, 2024

Conversation

erlend-aasland
Copy link
Contributor
@erlend-aasland erlend-aasland commented Feb 2, 2024 •
edited
Loading

Refactor state_modulename_name() of the parsing state machine, by
adding helpers for the sections that deal with ...:

  1. parsing the function name
  2. normalizing the function kind
  3. dealing with cloned functions
  4. resolving return converters
  5. adding the function to the DSL parser

@erlend-aasland
pythongh-114258: Refactor Argument Clinic function name parser
aac1a76 
Yak-shaving in preparation for adding support for slots.

- factor out return converter resolver
- factor out cloning
@bedevere-app bedevere-app bot mentioned this pull request Feb 2, 2024
Open
@erlend-aasland erlend-aasland marked this pull request as draft February 2, 2024 18:59
@erlend-aasland
Copy link
Contributor Author

(I'll fix the mypy errors later)

@erlend-aasland

This comment was marked as resolved.

Sign in to view
@erlend-aasland erlend-aasland marked this pull request as ready for review February 3, 2024 01:15
@erlend-aasland
Copy link
Contributor Author

FTR, most of this PR is just shuffling lines around and wrapping them in more targeted scopes.

AlexWaygood
AlexWaygood reviewed Feb 5, 2024
View reviewed changes
sobolevn
sobolevn reviewed Feb 5, 2024
View reviewed changes
Copy link
Member
@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Quick review :)

@erlend-aasland
Copy link
Contributor Author

I addressed your remarks in be244f2, @sobolevn :)

@sobolevn
Copy link
Member
sobolevn commented Feb 5, 2024 •
edited
Loading

Did you reject #114930 (comment) or missed it? :)

Update: Ok, I see that it was partially implemented :)

sobolevn
sobolevn approved these changes Feb 5, 2024
View reviewed changes
Copy link
Member
@sobolevn sobolevn left a comment

Choose a reason for hiding this comment

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

Thanks! We can work on better error messages in the next PRs.

@erlend-aasland
Copy link
Contributor Author

Thanks! We can work on better error messages in the next PRs.

Yeah, I think we should try to do an audit of all the fail(...) calls. Let's open an issue.

@erlend-aasland
Copy link
Contributor Author

Thanks for the review, Nikita. I'll wait a day or two to give Alex a chance to chime in (since I already pinged him about this).

@erlend-aasland erlend-aasland mentioned this pull request Feb 6, 2024
Open
@erlend-aasland
Copy link
Contributor Author

Thanks for the review, Nikita. I'll wait a day or two to give Alex a chance to chime in (since I already pinged him about this).

Landing this tomorrow afternoon (CET).

@erlend-aasland erlend-aasland merged commit 32f8ab1 into python:main Feb 15, 2024
@erlend-aasland erlend-aasland deleted the clinic/refactor-parse-func branch February 15, 2024 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AlexWaygood AlexWaygood AlexWaygood left review comments

@sobolevn sobolevn sobolevn approved these changes

@vstinner vstinner Awaiting requested review from vstinner

Assignees
No one assigned
Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@erlend-aasland @sobolevn @AlexWaygood
0