-
Notifications
You must be signed in to change notification settings - Fork 59.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update linking-a-pull-request-to-an-issue.md #199
Conversation
Fixes github#165 . I added a Note box at the end of the document, and a reference to it at the top of the document - should be enough warning without disrupting the rest of the text. Also changed the introdutory line in the "Linking a pull request(...)" section as suggested by the author.
Amazing @casals! I'll send this one to @github/docs-content-core team to review, too 👍 |
Included callout tag for note. Moved note to the beginning of the text and removed the related observation in the next paragraph since it became redundant.
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.
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.
Hi - thanks for addressing this issue, it's great to have this topic improved ✨
I'm a little puzzled by:
I added a Note box at the end of the document, and a reference to it at the top of the document
On staging, the note box appears to be at the start of the topic. Given that our search links to each H3 within the topic, I'm wondering if the note would be better placed within the "Linking a pull request to an issue using a keyword" section, maybe straight after the bulleted list of special terms. Otherwise people who go straight to that section will miss the note. What do you think?
If we did move the note box, then probably we should expand the first sentence of the topic: "You can link an issue to a pull request manually or using a supported keyword in the pull request description." to mention the default branch.
Hey @felicitymay - what happened here was: I had just submitted a number of PRs that involved including a Note box at the end of the document. During the review of the first of these PRs, one of the requested changes was to move the Note box to the top of the page, since that's the standard style/practice. In order to maintain consistency, I moved the (new) Note box to the top of the document in all of my PRs - but forgot to change the description here, sorry. All that being said, I think we can go with your latter suggestion: keep the box at the topic of the document + expand the first sentence of the topic. In fact, this sentence was already extended from its original version. I'm updating the PR description with these changes. |
Updated with requested changes
content/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue.md
Outdated
Show resolved
Hide resolved
…uest-to-an-issue.md Updated with requested changes Co-authored-by: Felicity Chapman <[email protected]>
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 👍🏻 🚀
Thanks for responding so quickly - it's much appreciated. I've added the |
Ah. Okay. Will check back to see when the tests have passed and merge. |
Thanks very much for contributing! Your pull request has been merged 🎉 You should see your changes appear on the site in approximately 24 hours. If you haven't already, you can add yourself to the list of contributors by creating a new comment in this PR using these instructions. Thanks again! ✨ |
Why:
Fixes #165 .
What's being changed:
I added a Note box at the top of the document, and a reference to it at the top of the document - should be enough warning without disrupting the rest of the text. Also changed the introductory line in the "Linking a pull request(...)" section as suggested by the author/PR reviewer:
Check off the following: