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-115756: make PyCode_GetFirstFree an unstable API by wrongnull · Pull Request #115781 · python/cpython

Conversation

@wrongnull

@wrongnull wrongnull commented Feb 21, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

According to this , should I mention it in the documentation in a separate pull request?

  • Issue: Make PyCode_GetFirstFree a PEP-689 unstable API #115756

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@wrongnull
make PyCode_GetFirstFree an unstable API
cf33af7
@wrongnull
wrongnull requested review from gvanrossum, iritkatriel and markshannon as code owners February 21, 2024 20:33
@bedevere-app bedevere-app Bot added the awaiting review label Feb 21, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 21, 2024
Make PyCode_GetFirstFree a PEP-689 unstable API #115756
Closed
@Eclips4
Eclips4 requested review from encukou, erlend-aasland and vstinner and removed request for gvanrossum February 21, 2024 20:34
@erlend-aasland

erlend-aasland commented Feb 21, 2024

Copy link
Copy Markdown
Contributor

According to this , should I mention it in the documentation in a separate pull request?

No; this PR represents an atomic change. Documentation (and preferably tests) follow. Your previous PR consisted of two different changes1.

But you cannot just remove the existing API; you need to keep it and deprecate it (see PEP-387).

Footnotes

  1. 1: fix the return type of one API. 2: change the documentation of another API. ↩
All reactions

Sorry, something went wrong.

Uh oh!

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

Eclips4 commented Feb 22, 2024

Copy link
Copy Markdown
Member

Here's an example of deprecation: #105397
wrongnull 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.
wrongnull added 2 commits February 23, 2024 00:55
@wrongnull
Preserve deprecated API function
356db2f
@wrongnull
Make necessary changes in a documentation
001fe51
@wrongnull

wrongnull commented Feb 22, 2024

Copy link
Copy Markdown
Contributor Author

The question may seem silly but should I add a news section?
All reactions

Sorry, something went wrong.

Uh oh!

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

encukou commented Feb 28, 2024

Copy link
Copy Markdown
Member

Yes, you should. it's a user-visible change.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
wrongnull and others added 2 commits February 29, 2024 10:43
@wrongnull
Merge branch 'main' into raname-getfirstfree
db8cb38
@wrongnull
add news entry
012f351
@encukou encukou mentioned this pull request Mar 8, 2024
Make PyCode_GetFirstFree unstable capi-workgroup/decisions#18
Closed
5 tasks done
encukou
encukou reviewed Mar 19, 2024
View reviewed changes
Comment thread Doc/whatsnew/3.13.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@encukou
Avoid undefined reference
1a45b38
@encukou
encukou enabled auto-merge (squash) March 19, 2024 08:31
encukou
encukou approved these changes Mar 19, 2024
View reviewed changes
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Mar 19, 2024
@encukou
Add a blurb
04c101d
vstinner
vstinner reviewed Mar 19, 2024
View reviewed changes
Comment thread Include/cpython/code.h
assert(PyCode_Check(op));
return op->co_nlocalsplus - op->co_nfreevars;
}

@vstinner vstinner Mar 19, 2024 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If the deprecation has no detail, for me, it can be removed as soon as in Python 3.15. But the doc is more specific:

"The old name is deprecated, but will remain available until the signature changes again."

Please add a comment to explain it here.

Sorry, something went wrong.

Uh oh!

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

@encukou encukou Mar 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If the deprecation has no detail, for me, it can be removed in Python 3.15.

What are you basing this on?

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Mar 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

PEP 387: Similarly a feature cannot be removed without notice between any two consecutive releases.

Sorry, something went wrong.

Uh oh!

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

@encukou encukou Mar 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I'm sorry, I didn't disable automerge after your comment. I didn't mean to ignore you :(

But IMO, the “Basic Policy” is just a summary. The “Making Incompatible Changes” section has more detailed instructions.
If you disagree with my reading, let's take it to Discourse; this isn't a good place for it.

Sorry, something went wrong.

Uh oh!

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

@vstinner vstinner Mar 19, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

It's just that this function has a specific comment in the doc, IMO it's worth it to repeat it in the header to avoid future eagger removal. But well, now that the change is merged, it's maybe no longer worth it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
encukou reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@encukou
encukou merged commit a8e93d3 into python:main Mar 19, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 19, 2024
vstinner pushed a commit to vstinner/cpython that referenced this pull request Mar 20, 2024
@wrongnull @vstinner
pythongh-115756: make PyCode_GetFirstFree an unstable API (pythonGH-1…
9ca3e2c 
…15781)
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
@wrongnull @adorilson
pythongh-115756: make PyCode_GetFirstFree an unstable API (pythonGH-1…
245bdd5 
…15781)
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
@wrongnull @diegorusso
pythongh-115756: make PyCode_GetFirstFree an unstable API (pythonGH-1…
40f3cf7 
…15781)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner left review comments
@encukou encukou encukou approved these changes
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner
@iritkatriel iritkatriel Awaiting requested review from iritkatriel iritkatriel is a code owner
@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland

Uh oh!

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

Assignees

No one assigned

Labels

None yet

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.

5 participants

@wrongnull @erlend-aasland @Eclips4 @encukou @vstinner
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.