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 — Change links to label refs by slateny · Pull Request #98454 · python/cpython

Conversation

@slateny

@slateny slateny commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

No description provided.

Sorry, something went wrong.

Uh oh!

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

SautPutra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@slateny
Change links to label refs
bbf7381
@slateny
slateny requested a review from ericvsmith as a code owner October 19, 2022 15:45
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Oct 19, 2022
@slateny slateny added the skip issue label Oct 19, 2022
slateny
slateny commented Oct 19, 2022
View reviewed changes
Comment thread Doc/howto/isolating-extensions.rst
@@ -461,7 +461,7 @@ Module State Access from Slot Methods, Getters and Setters

.. After adding to limited API:

@slateny slateny Oct 19, 2022

Copy link
Copy Markdown
Contributor Author

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.

This part doesn't render (see page) so unsure what this should look like.

Sorry, something went wrong.

Uh oh!

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

All reactions

@CAM-Gerlach CAM-Gerlach Oct 19, 2022

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 a reST "comment", which doesn't render; not sure if its intended to be or not, since it does use reST syntax within itself.

@encukou any insight here?

Sorry, something went wrong.

Uh oh!

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

All reactions
@slateny
slateny requested a review from CAM-Gerlach October 19, 2022 15:47
CAM-Gerlach
CAM-Gerlach reviewed Oct 19, 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.

Thanks @slateny ; a couple comments.

At least some of these are likely because things were converted from PEPs, and should be less common now that we added Intersphinx support there.

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Doc/howto/isolating-extensions.rst
@@ -461,7 +461,7 @@ Module State Access from Slot Methods, Getters and Setters

.. After adding to limited API:

@CAM-Gerlach CAM-Gerlach Oct 19, 2022

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 a reST "comment", which doesn't render; not sure if its intended to be or not, since it does use reST syntax within itself.

@encukou any insight here?

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread Doc/reference/datamodel.rst Resolved

Uh oh!

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

Comment thread Doc/library/dataclasses.rst Resolved

Uh oh!

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

slateny and others added 2 commits October 20, 2022 09:10
@slateny
Add missed label and ref
7613ba7
@slateny @CAM-Gerlach
Use more specific label name
20fa1fb
Co-authored-by: C.A.M. Gerlach <[email protected]>
@slateny

slateny commented Oct 20, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

Can't reach it with inline comments, but there are also links like here (linking to 3.4 whatsnew changelog) that I'm not too sure if it can be converted into a label. With a changelog label, such as in the 3.7 whatsnew, if not on the right version on the webpage (.org/3/ instead of .org/3.7), the changelog link goes to the most current one (3.10 ish), while in the hardlinked whatsnew, such as the 3.6 whatsnew, changelog goes to the right page.

All reactions

Sorry, something went wrong.

Uh oh!

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

@CAM-Gerlach CAM-Gerlach added needs backport to 3.10 needs backport to 3.11 only security fixes labels Oct 20, 2022
@CAM-Gerlach

CAM-Gerlach commented Oct 20, 2022

Copy link
Copy Markdown
Member

With a changelog label, such as in the 3.7 whatsnew, if not on the right version on the webpage (.org/3/ instead of .org/3.7), the changelog link goes to the most current one (3.10 ish), while in the hardlinked whatsnew, such as the 3.6 whatsnew, changelog goes to the right page.

Instead of linking to a whole different docs build, what should instead be done is to tweak blurb here to add standard ref target labels for each section (and subsection) when generating the news file based on the version tag, e.g. 3.11.0-alpha-1 or 3.11.0 for the final. Then, the appropriate sections for each version, etc., can be linked. However, that would be best addressed in a separate PR after the change to blurb is made and deployed, assuming it is accepted.

There are a few remaining relevant instances that can be fixed in this PR:

Doc/using/venv-create.inc:20:   <https://docs.python.org/dev/whatsnew/3.6.html#id8>`_.
Doc/whatsnew/2.2.rst:27:Reference <https://docs.python.org/2.2/lib/lib.html>`_ and the `Python
Doc/whatsnew/2.2.rst:28:Reference Manual <https://docs.python.org/2.2/ref/ref.html>`_.  If you want to
Doc/whatsnew/2.2.rst:398:https://docs.python.org/dev/howto/descriptor.html is a lengthy tutorial introduction to
Misc/NEWS.d/3.9.0a3.rst:809:<https://docs.python.org/3/library/inspect.html#types-and-members>`_

At some point, if we add pre-commit checks to this repo, we could add a check for these links, like I added for hardcoded links to PEPs and RFCs on the PEPs repo (which there also are a handful of here, BTW, and could be fixed in a separate PR).

slateny 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.

@slateny
Change some more links to labels
9fe6bfd
@slateny
slateny requested a review from vsajip as a code owner October 21, 2022 01:31
@slateny

slateny commented Oct 21, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor Author

I left out

Doc/whatsnew/2.2.rst:27:Reference <https://docs.python.org/2.2/lib/lib.html>`_ and the `Python
Doc/whatsnew/2.2.rst:28:Reference Manual <https://docs.python.org/2.2/ref/ref.html>`_.  If you want to

for now as the pages are far too different to the corresponding modern links, and uncertain if those should be changed over or if they should just remain for historical purposes (as those pages seem to remain for historical purposes too).

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 JelleZijlstra and removed request for ericvsmith and vsajip October 21, 2022 03:28
@JelleZijlstra

JelleZijlstra commented Oct 21, 2022

Copy link
Copy Markdown
Member

Yes, let's leave the old what's news unchanged as much as possible.

All reactions

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach
CAM-Gerlach approved these changes Oct 21, 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.

LGTM, thanks @slateny

Sorry, something went wrong.

Uh oh!

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

slateny reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Doc/whatsnew/3.6.rst Outdated
The :mod:`tkinter.tix` module is now deprecated. :mod:`tkinter` users
should use :mod:`tkinter.ttk` instead.

.. _whatsnew-3.6-venv:

@CAM-Gerlach CAM-Gerlach Oct 21, 2022

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.

Sidenote: Following the convention I've used for the 3.11 What's New would be whatsnew36-venv, but I think this convention (or at least whatsnew3.6-venv) would be better—out of old habit I often avoid using . in ref target labels. In any case, no practical reason to change it here.

Sorry, something went wrong.

Uh oh!

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

slateny reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@CAM-Gerlach CAM-Gerlach Oct 21, 2022

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.

Sorry for the confusion—I was mentioning this as a sidenote and suggestion you not change it here (instead, I really should be using it in the ref targets I'm adding, though probably too late now for 3.11—I can adopt it with 3.12). But since you have changed it, not really worth changing it again.

Just to be more clear for the future, if I'm actually requesting a change, I will pretty much always make it as a actual suggestion (unless its impractical or impossible), so you can easily apply it with Files -> Add to batch -> Commit rather than manually implementing, committing, pushing and resolving it. The way I see it is if I'm going to ask someone else to make a change on their PR (unless its really important), its only fair that I put in the work myself to implement it, so all they have to do is click a button to accept it.

Sorry, something went wrong.

Uh oh!

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

All reactions

@slateny slateny Oct 22, 2022

Copy link
Copy Markdown
Contributor Author

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 see I see, don't worry too much about it - I checked the existing labels and just decided that it would be good to follow convention. And conversely, I requested the review so I definitely don't mind the work (and I would have to run it locally anyway just to check the rendering), but of course the ease of changing is always appreciated.

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread Misc/NEWS.d/3.9.0a3.rst Outdated
Add an entry for ``__module__`` in the "function" & "method" sections of the
`inspect docs types and members table
<https://docs.python.org/3/library/inspect.html#types-and-members>`_
:ref:`inspect docs types and members table <inspect-types>`.

@CAM-Gerlach CAM-Gerlach Oct 21, 2022

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 a bit cleaner to do

Suggested change
:ref:`inspect docs types and members table <inspect-types>`.
:mod:`inspect` docs :ref:`inspect-types` table.

Sorry, something went wrong.

Uh oh!

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

slateny reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Oct 21, 2022
JelleZijlstra
JelleZijlstra approved these changes Oct 21, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Oct 21, 2022
@CAM-Gerlach

CAM-Gerlach commented Oct 21, 2022

Copy link
Copy Markdown
Member

I left out for now as the pages are far too different to the corresponding modern links, and uncertain if those should be changed over or if they should just remain for historical purposes (as those pages seem to remain for historical purposes too).

Yeah, not much point changing those

All reactions

Sorry, something went wrong.

Uh oh!

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

@slateny
Follow convention for label naming, add link to inspect module
6abd61c
CAM-Gerlach
CAM-Gerlach approved these changes Oct 22, 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.

LGTM, thanks @slateny

Sorry, something went wrong.

Uh oh!

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

All reactions
@JelleZijlstra
JelleZijlstra merged commit 268129a into python:main Oct 26, 2022
@miss-islington

miss-islington commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

Thanks @slateny for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the awaiting merge label Oct 26, 2022
@miss-islington

miss-islington commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

Sorry, @slateny and @JelleZijlstra, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 268129a74f01adb7bb14cd71d1f38378e39d304d 3.11

All reactions

Sorry, something went wrong.

Uh oh!

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

@miss-islington miss-islington assigned JelleZijlstra Oct 26, 2022
@miss-islington

miss-islington commented Oct 26, 2022

Copy link
Copy Markdown
Contributor

Sorry @slateny and @JelleZijlstra, I had trouble checking out the 3.10 backport branch.
Please retry by removing and re-adding the "needs backport to 3.10" label.
Alternatively, you can backport using cherry_picker on the command line.
cherry_picker 268129a74f01adb7bb14cd71d1f38378e39d304d 3.10

All reactions

Sorry, something went wrong.

Uh oh!

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

@JelleZijlstra

JelleZijlstra commented Oct 26, 2022

Copy link
Copy Markdown
Member

@slateny would you be able to do the backports?

All reactions

Sorry, something went wrong.

Uh oh!

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

@slateny

slateny commented Oct 26, 2022

Copy link
Copy Markdown
Contributor Author

Yep sure

JelleZijlstra reacted with hooray emoji
All reactions
  • 🎉 1 reaction

Sorry, something went wrong.

Uh oh!

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

@slateny
slateny deleted the s/remove-hard-links branch October 26, 2022 14:37
slateny added a commit to slateny/cpython that referenced this pull request Oct 26, 2022
@slateny @CAM-Gerlach
docs: Change links to label refs (python#98454)
1156b8d
Co-authored-by: C.A.M. Gerlach <[email protected]>
(cherry picked from commit 268129a)
@bedevere-bot

bedevere-bot commented Oct 26, 2022

Copy link
Copy Markdown

GH-98725 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Oct 26, 2022
slateny added a commit to slateny/cpython that referenced this pull request Oct 26, 2022
@slateny @CAM-Gerlach
[3.10] Change links to label refs (pythonGH-98454)
2a3594f
Co-authored-by: C.A.M. Gerlach <[email protected]>
slateny added a commit to slateny/cpython that referenced this pull request Oct 26, 2022
@slateny @CAM-Gerlach
[3.10] Change links to label refs (pythonGH-98454)
c43c788
Co-authored-by: C.A.M. Gerlach <[email protected]>
@bedevere-bot

bedevere-bot commented Oct 26, 2022

Copy link
Copy Markdown

GH-98726 is a backport of this pull request to the 3.10 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

@bedevere-bot bedevere-bot removed the needs backport to 3.10 label Oct 26, 2022
JelleZijlstra pushed a commit that referenced this pull request Oct 26, 2022
@slateny @CAM-Gerlach
[3.11] Change links to label refs (GH-98454) (#98725)
5f86275
Co-authored-by: C.A.M. Gerlach <[email protected]>
(cherry picked from commit 268129a)
JelleZijlstra pushed a commit that referenced this pull request Oct 26, 2022
@slateny @CAM-Gerlach
[3.10] Change links to label refs (GH-98454) (#98726)
f786485
Co-authored-by: C.A.M. Gerlach <[email protected]>
jaraco pushed a commit to python/importlib_metadata that referenced this pull request Apr 15, 2023
@slateny @CAM-Gerlach @jaraco
docs: Change links to label refs (python/cpython#98454)
1cf7385
Co-authored-by: C.A.M. Gerlach <[email protected]>
clrpackages pushed a commit to clearlinux-pkgs/pypi-importlib_metadata that referenced this pull request Apr 18, 2023
@fenrus75
pypi-importlib_metadata: Autospec creation for update from version 6.…
10ae339
…3.0 to version 6.4.1

Jason R. Coombs (5):
      Re-use _path.build for building files.
      Re-use FilesSpec from _path.
      Add type hint to read_text result.
      Update changelog
      Update changelog

Paul Watson (1):
      gh-102354: change python3 to python in docs examples (python/cpython#102696)

Stanley (1):
      docs: Change links to label refs (python/cpython#98454)
erlend-aasland pushed a commit to erlend-aasland/devguide that referenced this pull request Sep 8, 2023
@slateny @CAM-Gerlach @erlend-aasland
docs: Change links to label refs (python/cpython#98454)
c8b7d15
Co-authored-by: C.A.M. Gerlach <[email protected]>
AA-Turner pushed a commit to AA-Turner/devguide that referenced this pull request Sep 13, 2023
@slateny @CAM-Gerlach
docs: Change links to label refs (python/cpython#98454)
1196c45
Co-authored-by: C.A.M. Gerlach <[email protected]>
erlend-aasland pushed a commit to python/devguide that referenced this pull request Sep 26, 2023
@slateny @CAM-Gerlach @erlend-aasland
docs: Change links to label refs (python/cpython#98454)
d49557b
Co-authored-by: C.A.M. Gerlach <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes
@CAM-Gerlach CAM-Gerlach CAM-Gerlach approved these changes

Uh oh!

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

Assignees

@JelleZijlstra JelleZijlstra

Labels

docs Documentation in the Doc dir skip issue 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.

5 participants

@slateny @CAM-Gerlach @JelleZijlstra @miss-islington @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.