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-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix by wjakob · Pull Request #99056 · python/cpython

Conversation

@wjakob

@wjakob wjakob commented Nov 3, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

PR #98587 addressing issue #98586 lacked a "what's new" entry.

While making those changes, I noticed an inconsistency in how PY_VECTORCALL_ARGUMENTS_OFFSET is declared in the underlying Sphinx markup when compared to other macro constants like Py_TPFLAGS_HAVE_VECTORCALL. This PR fixes that as well, which should connect a few currently broken cross-references

  • Issue: Add vector call functions to the limited API #98586

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@wjakob
pythongh-98586: add whatsnew entry, related documentation fix
11b4bca 
PR python#98587 addressing issue python#98586 lacked a "what's new" entry.

While making those changes, I noticed an inconsistency in how
``PY_VECTORCALL_ARGUMENTS_OFFSET`` is declared in the underlying Sphinx
markup when compared to other macro constants like
``Py_TPFLAGS_HAVE_VECTORCALL``. This PR fixes that as well, which should
connect a few currently broken cross-references
@bedevere-bot bedevere-bot mentioned this pull request Nov 3, 2022
Add vector call functions to the limited API #98586
Closed
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir labels Nov 3, 2022
@wjakob
removed repeated word
def3757
@CAM-Gerlach

CAM-Gerlach commented Nov 3, 2022

Copy link
Copy Markdown
Member

FYI, you should mention in the PR title (which is also the default commit message) what you are adding a What's New entry for...
wjakob and erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
CAM-Gerlach
CAM-Gerlach reviewed Nov 3, 2022
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment

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.

Seems like these should be changed to use equivalent roles from the appropriate domain, though I'm not sure why the Python domain has data/const but the C domain doesn't, despite C actually having "real" constants and Python only having a convention, nor what the reason was for using construct from the "wrong" domain originally...

@AA-Turner , any expert insight here?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/c-api/call.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Misc/NEWS.d/next/Core and Builtins/2022-10-24-10-30-30.gh-issue-98586.Tha5Iy.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@wjakob wjakob changed the title gh-98586: add whatsnew entry, related documentation fix gh-98586: add whatsnew entry covering limited-API outgoing vector calls, related documentation fix Nov 3, 2022
@wjakob @CAM-Gerlach
Add extra article
c493411 
Co-authored-by: C.A.M. Gerlach <[email protected]>
@wjakob

wjakob commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

@CAM-Gerlach -- I updated the PR title (sorry) and merged the change with the missing article.

I am hesitant to make the domain-related changes you requested. Part of the reason is that I am following the pattern that generally seems to be used for macro constants. For example look at how Py_TPFLAGS_MANAGED_DICT is defined and referenced in the Sphinx doc (or any macro constant, as far as I can tell). If you wanted to move things over to the :c: Sphinx domain, then it would be good to do this in a concerted set of changes covering the entire documentation.
erlend-aasland 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.
CAM-Gerlach
CAM-Gerlach approved these changes Nov 3, 2022
View reviewed changes

@CAM-Gerlach CAM-Gerlach left a comment

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.

Okay, that makes sense for now I guess. In that case, LGTM, aside from one comment

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Nov 3, 2022
@wjakob @CAM-Gerlach
Update Doc/whatsnew/3.12.rst
862307c 
Co-authored-by: C.A.M. Gerlach <[email protected]>
@wjakob

wjakob commented Nov 3, 2022

Copy link
Copy Markdown
Contributor Author

aside from one comment

Incorporated, thanks.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@CAM-Gerlach
CAM-Gerlach requested review from encukou, erlend-aasland and markshannon November 3, 2022 22:43
erlend-aasland
erlend-aasland reviewed Nov 4, 2022
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
wjakob added 2 commits November 4, 2022 12:36
@wjakob
fixed referenced macro constant
c7517df
@wjakob
rephrase
4d0c68b
erlend-aasland
erlend-aasland reviewed Nov 4, 2022
View reviewed changes
Comment thread Doc/whatsnew/3.12.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@wjakob @erlend-aasland
Update Doc/whatsnew/3.12.rst
c55a974 
Co-authored-by: Erlend E. Aasland <[email protected]>
erlend-aasland
erlend-aasland approved these changes Nov 4, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

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.

LGTM, thanks!

Related: we should really update the docs on best practice for writing exension modules using the Limited C API (there is already an issue for this).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Nov 4, 2022
@erlend-aasland
erlend-aasland merged commit 57a4052 into python:main Nov 6, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 6, 2022
@wjakob
wjakob deleted the whatsnew branch November 14, 2022 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CAM-Gerlach CAM-Gerlach CAM-Gerlach approved these changes
@erlend-aasland erlend-aasland erlend-aasland approved these changes
@markshannon markshannon Awaiting requested review from markshannon
@encukou encukou Awaiting requested review from encukou

Uh oh!

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

Assignees

No one assigned

Labels

docs Documentation in the Doc dir

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

@wjakob @CAM-Gerlach @encukou @erlend-aasland @bedevere-bot
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.