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

gh-103092: Adapt _lsprof methods to Argument Clinic #115242

Closed
wants to merge 8 commits into from

Conversation

aisk
Copy link
Member

@aisk aisk commented Feb 10, 2024

This is split from #115130 as pre-processing to isolate the _lsprof module.

Some methods have added a cls: defining_class parameter to their signature but do not use it. This is because #115130 will use it to passing module state.

@erlend-aasland

@erlend-aasland
Copy link
Contributor

A couple of remarks after a quick glance1:

  • It seems there are many similar signatures; consider using the AC "clone" feature
  • The defining class converter should be marked as positional-only (/)
  • Style nit: can you use a more concise clinic input block:
    • drop the extra newline between /*[clinic input] and the function def
    • drop the extra newline between the function def and the parameter section

Footnotes

  1. I'll be able to do a review tomorrow or Monday

@erlend-aasland
Copy link
Contributor

Closing, as #115130 was superseded by #115301.

@aisk aisk deleted the lsprof-clinic branch March 1, 2024 05:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants