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 — Document auto-closing keywords for PRs. by ezio-melotti · Pull Request #855 · python/devguide

Conversation

@ezio-melotti

@ezio-melotti ezio-melotti commented May 6, 2022

Copy link
Copy Markdown
Member

Fixes now outdated #502 and supersedes #701.

Sorry, something went wrong.

Uh oh!

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

All reactions
@ezio-melotti
Document auto-closing keywords for PRs.
48ffd3b
@ezio-melotti ezio-melotti added the type-feature Additions; New content or section needed label May 6, 2022
@ezio-melotti ezio-melotti self-assigned this May 6, 2022
@ezio-melotti

ezio-melotti commented May 6, 2022

Copy link
Copy Markdown
Member Author

See https://cpython-devguide--855.org.readthedocs.build/gitbootcamp/#creating-a-pull-request

All reactions

Sorry, something went wrong.

Uh oh!

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

hugovk
hugovk approved these changes May 6, 2022
View reviewed changes
Comment thread gitbootcamp.rst Resolved

Uh oh!

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

AlexWaygood
AlexWaygood reviewed May 6, 2022
View reviewed changes
Comment thread gitbootcamp.rst Resolved

Uh oh!

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

methane
methane reviewed May 6, 2022
View reviewed changes
Comment thread gitbootcamp.rst Resolved

Uh oh!

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

ezio-melotti added 2 commits July 12, 2022 03:17
@ezio-melotti
Merge branch 'main' into autoclosing-kws
e8f7e6a
@ezio-melotti
Bedevere now adds the issue number in the body.
34d2080
CAM-Gerlach
CAM-Gerlach reviewed Jul 12, 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.

A couple textual suggestions

Sorry, something went wrong.

Uh oh!

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

All reactions
Comment thread getting-started/git-boot-camp.rst Resolved

Uh oh!

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

Comment thread getting-started/git-boot-camp.rst Resolved

Uh oh!

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

@ezio-melotti @CAM-Gerlach
Apply suggestions from code review
7a799a2
Co-authored-by: CAM Gerlach <[email protected]>
@ezio-melotti

ezio-melotti commented Jul 13, 2022

Copy link
Copy Markdown
Member Author

I applied the suggested changes, and we now have 3 options:

  1. merge as is
  2. clarify when they should/shouldn't be used
  3. avoid documenting them
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 Aug 16, 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.

Sorry for the long delay—missed this somehow.

The text is LGTM, for what it's worth. It might be helpful to have additional clarity on when they should/shouldn't be used, but that's above my pay grade, as is the overall decision on this PR, of course.

Sorry, something went wrong.

Uh oh!

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

ezio-melotti reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@CAM-Gerlach
CAM-Gerlach requested review from AlexWaygood and methane August 16, 2022 01:30
AlexWaygood
AlexWaygood reviewed Aug 16, 2022
View reviewed changes
Comment thread getting-started/git-boot-camp.rst Outdated
Comment on lines +291 to +293
to automatically close the linked issue once the PR is merged. However, you
should avoid them if other PRs need to be merged before the issue can be
closed (e.g. backports to other branches).

@AlexWaygood AlexWaygood Aug 16, 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.

For a new contributor, this might raise more questions than it answers. Can we link to a reference somewhere else in the devguide that tells contributors which PRs are generally backported, and which are not?

Sorry, something went wrong.

Uh oh!

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

All reactions

@CAM-Gerlach CAM-Gerlach Aug 16, 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.

To note, there's also other benefits to linking a PR beyond just auto-close—most notably, it adds an easy to find reference to the PR from the issue and the issue from the PR a number of handy places in the GitHub UI, which make navigation a fair bit quicker than manually/semi-automatically trying to figure out the corresponding PR (or issue).

Sorry, something went wrong.

Uh oh!

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

All reactions

@terryjreedy terryjreedy Aug 16, 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.

Nearly all bug fixes, doc fixes, and IDLE changes are backported, and nearly all feature changes are not. But even feature issues may require multiple PRs, so the above advice is still incomplete. And there are PRs without an issue. By linking autoclose to linking of issue to PR, GH has made issue-PR linking worse than useless.

Sorry, something went wrong.

Uh oh!

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

All reactions

@CAM-Gerlach CAM-Gerlach Aug 17, 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.

Yeah, unfortunately the story on GitHub around relationships of multiple PRs for one issue has never been good, and unlike a number of other pain points that have improved substantially in recent years, this one doesn't seem to have. And unlike UI issues, there's not as much Refined GitHub and tools like it can do about it—it is a big reason that GitHub overlays like ZenHub gained such a following

Sorry, something went wrong.

Uh oh!

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

All reactions

@ezio-melotti ezio-melotti Aug 18, 2022

Copy link
Copy Markdown
Member 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.

unfortunately the story on GitHub around relationships of multiple PRs for one issue has never been good

I created this:

  • Add PR links to the issue bedevere#501

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach and AlexWaygood reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

@omerbensaadon omerbensaadon Sep 23, 2022 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown

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.

hey folks, I've captured this feedback! I hope that [tasklists] can help make this experienc better in the future. keep it coming :)

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach, AlexWaygood, and ezio-melotti reacted with hooray emoji
All reactions
  • 🎉 3 reactions

@CAM-Gerlach CAM-Gerlach Sep 23, 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.

Thanks @omerbensaadon ! We all really appreciate it!

Sorry, something went wrong.

Uh oh!

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

All reactions
@ezio-melotti
Add a new entry about linking to issues/PRs.
a279ae1
@ezio-melotti

ezio-melotti commented Aug 18, 2022

Copy link
Copy Markdown
Member Author

I added a separate entry to the bootcamp about linking, including a rephrased version of the paragraph included in the previous commits.

All reactions

Sorry, something went wrong.

Uh oh!

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

AlexWaygood
AlexWaygood reviewed Aug 18, 2022
View reviewed changes
Comment thread getting-started/git-boot-camp.rst Resolved

Uh oh!

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

CAM-Gerlach
CAM-Gerlach reviewed Aug 18, 2022
View reviewed changes
Comment thread getting-started/git-boot-camp.rst Resolved

Uh oh!

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

CAM-Gerlach
CAM-Gerlach reviewed Aug 18, 2022
View reviewed changes
Comment thread getting-started/git-boot-camp.rst

You can link to issues and pull requests using ``gh-NNNNN`` (this form is
preferred over ``#NNNNN``). If the reference appears in a list, the link
will be expanded to show the status and title of the issue/PR.

@CAM-Gerlach CAM-Gerlach Aug 18, 2022 •
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.

Oh, is that really what triggers this? I've gotten inconsistent results in my resent testing, with links sometimes expanding and sometimes not in and outside of lists, even in the same spot. I'd be very curious to know.

Also, gh (and #?) references in commit messages get expanded to pythongh—how does that happen?

Sorry, something went wrong.

Uh oh!

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

All reactions

@ezio-melotti ezio-melotti Aug 19, 2022

Copy link
Copy Markdown
Member 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.

Oh, is that really what triggers this?

I think so, and it took me a while to figure that out too. ISTM that references like #855 are consistently expanded in lists like:

  • Document auto-closing keywords for PRs. #855
  • with some text Document auto-closing keywords for PRs. #855
  1. Document auto-closing keywords for PRs. #855
  2. with some text Document auto-closing keywords for PRs. #855
  • Document auto-closing keywords for PRs. #855
  • with some text Document auto-closing keywords for PRs. #855

But not in paragraphs like this (#855) or quotes like

#855

Do you have any counterexamples?

Also, gh (and #?) references in commit messages get expanded to pythongh—how does that happen?

I've seen that and I think it's just a bug in GitHub.

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach reacted with heart emoji
All reactions
  • ❤️ 1 reaction

@CAM-Gerlach CAM-Gerlach Aug 20, 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.

with some text #855

Ah, I misinterpreted what you said originally to mean just bullets with no other text, when I was seeing them both with and without—silly me.

Do you have any counterexamples?

Not any others that I can reliably reproduce—I believe the behavior I saw (on the What's New issues) when I was trying to figure this out might have perhaps just been a combination of an issue where the link wasn't getting created as above when the linked issue hadn't been created when I loaded the page, and/or just not being totally sure what caused it when testing a number of things (particularly #NNN vs org/repo#NNN vs <full URL> vs. ).

Sorry, something went wrong.

Uh oh!

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

All reactions

@CAM-Gerlach CAM-Gerlach Sep 12, 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.

One side note that came up during some related testing—AFAIK, GH-/gh- doesn't trigger the issue number lookup and autocompletion, which I find invaluable especially on repos nearing 6 digit issue and PR numbers like CPython.

Sorry, something went wrong.

Uh oh!

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

All reactions

@ezio-melotti ezio-melotti Oct 7, 2022

Copy link
Copy Markdown
Member 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.

In #946 (review) the ref is not expanded, maybe because it's in a review?

Screenshot showing both the unexpanded list, and the popup on the first ref which confirms the ref is correct:
image

Sorry, something went wrong.

Uh oh!

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

CAM-Gerlach reacted with eyes emoji
All reactions
  • 👀 1 reaction
@ezio-melotti @AlexWaygood
Make sentence more explicit.
30b5f7d
Co-authored-by: Alex Waygood <[email protected]>
AlexWaygood
AlexWaygood approved these changes Aug 21, 2022
View reviewed changes
@CAM-Gerlach CAM-Gerlach mentioned this pull request Sep 23, 2022
Tasklist test issue python/cpython#97021
Closed
@ezio-melotti
ezio-melotti merged commit 97a229f into python:main Oct 7, 2022
@ezio-melotti
ezio-melotti deleted the autoclosing-kws branch October 7, 2022 04:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arhadthedev arhadthedev arhadthedev left review comments
@CAM-Gerlach CAM-Gerlach CAM-Gerlach approved these changes
@terryjreedy terryjreedy terryjreedy left review comments
@AlexWaygood AlexWaygood AlexWaygood approved these changes
@hugovk hugovk hugovk approved these changes
@methane methane Awaiting requested review from methane
+1 more reviewer
@omerbensaadon omerbensaadon omerbensaadon left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

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

Assignees

@ezio-melotti ezio-melotti

Labels

type-feature Additions; New content or section needed

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.

8 participants

@ezio-melotti @methane @hugovk @arhadthedev @omerbensaadon @CAM-Gerlach @terryjreedy @AlexWaygood
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.