GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-113317: Argument Clinic: Add libclinic.function by vstinner · Pull Request #116807 · python/cpython

Conversation

@vstinner

@vstinner vstinner commented Mar 14, 2024 •
edited by erlend-aasland
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Move Module, Class, and Function and Parameter classes to a new libclinic.function module.
  • Move VersionTuple and Sentinels to libclinic.utils.
  • Issue: Argument Clinic: split out global stateless helpers and constants from clinic.py #113317

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@vstinner vstinner added the skip news label Mar 14, 2024
@vstinner
vstinner requested a review from erlend-aasland as a code owner March 14, 2024 13:08
@bedevere-app bedevere-app Bot added the awaiting core review label Mar 14, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Mar 14, 2024
Argument Clinic: split out global stateless helpers and constants from clinic.py #113317
Closed
@vstinner
pythongh-113317, AC: Add libclinic.function module
a330f0e 
Move Module, Class, Function and Parameter classes to a new
libclinic.function module.

Move VersionTuple and Sentinels to libclinic.utils.
@vstinner
vstinner force-pushed the clinic_parameter branch from 2da1479 to a330f0e Compare March 14, 2024 13:24
@vstinner
vstinner enabled auto-merge (squash) March 14, 2024 14:02
@vstinner
vstinner merged commit b1236a4 into python:main Mar 14, 2024
@vstinner
vstinner deleted the clinic_parameter branch March 14, 2024 14:37
@bedevere-app bedevere-app Bot removed the awaiting core review label Mar 14, 2024
@erlend-aasland erlend-aasland changed the title gh-113317, AC: Add libclinic.parameter gh-113317: Argument Clinic: Add libclinic.function Mar 19, 2024
@erlend-aasland

erlend-aasland commented Mar 19, 2024

Copy link
Copy Markdown
Contributor

Move Module, Class, Function and Parameter classes to a new libclinic.function module.

This naming does not make sense to me. Why would class, module and parameter live in a file called function.py?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Mar 19, 2024

Copy link
Copy Markdown
Member Author

This naming does not make sense to me. Why would class, module and parameter live in a file called function.py?

Do you want to propose a different name? Or have one class per file?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Mar 19, 2024

Copy link
Copy Markdown
Contributor

Do you want to propose a different name? Or have one class per file?

With one class per file you at least don't need to git grep in order to find out where things live; if the file is named after the class, it would be obvious where it was defined. Perhaps that is too granular, though.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Mar 19, 2024

Copy link
Copy Markdown
Contributor

The commit message says:

Move Module, Class, Function and Parameter classes to a new libclinic.function module.

But this is not true; Parameter was not moved; it was left out of the PR.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner

vstinner commented Mar 19, 2024

Copy link
Copy Markdown
Member Author

But this is not true; Parameter was not moved; it was left out of the PR.

Parameter was moved to libclinic.function in the merged commit, no? Search for class Parameter: in b1236a4. Well, I can be wrong.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@erlend-aasland

erlend-aasland commented Mar 19, 2024

Copy link
Copy Markdown
Contributor

No, you're right; I misread it in-between all the changes. Sorry!
vstinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner added a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@vstinner
pythongh-113317, AC: Add libclinic.function (python#116807)
9c3fc84 
Move Module, Class, Function and Parameter classes to a new
libclinic.function module.

Move VersionTuple and Sentinels to libclinic.utils.
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@vstinner @adorilson
pythongh-113317, AC: Add libclinic.function (python#116807)
b08aab3 
Move Module, Class, Function and Parameter classes to a new
libclinic.function module.

Move VersionTuple and Sentinels to libclinic.utils.
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@vstinner @diegorusso
pythongh-113317, AC: Add libclinic.function (python#116807)
564fb56 
Move Module, Class, Function and Parameter classes to a new
libclinic.function module.

Move VersionTuple and Sentinels to libclinic.utils.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@vstinner @erlend-aasland
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.