Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Alexandrammr - Adding steps in instructions for #2150 #6667

Merged
merged 13 commits into from
May 27, 2021
Merged

Alexandrammr - Adding steps in instructions for #2150 #6667

merged 13 commits into from
May 27, 2021

Conversation

AlexandraMMR
Copy link
Contributor

@AlexandraMMR AlexandraMMR commented May 22, 2021 •
edited
Loading

Why:

Closes #2150

What's being changed:

Adding additional steps to sections in Adding a new page to your site and Adding a new post to your site by adding the PR merge completion parts so the site is updated with changes.

Specifically, I have created new partial files (reusables) to keep in the style of the previous steps templates and added them in the 2 sections.

Check off the following:

  • I have reviewed my changes in staging (look for the deploy-to-heroku link in your pull request, then click View deployment).
  • For content changes, I have completed the self-review checklist.

Writer impact (This section is for GitHub staff members only):

  • This pull request impacts the contribution experience
    • I have added the 'writer impact' label
    • I have added a description and/or a video demo of the changes below (e.g. a "before and after video")

@github-actions github-actions bot added the triage Do not begin working on this issue until triaged by the team label May 22, 2021
@AlexandraMMR
Copy link
Contributor Author

AlexandraMMR commented May 22, 2021 •
edited
Loading

The deployment is looking good, however, there seems to be a bug with any pages that aren't under the /pages hierarchy, eg the Merge pull request link is under /github and seems that it can't access it 😞 Even happens when I do a search for the page so don't think it's an issue with the link, would appreciate it if you can have a look at it before approving. Thank you!

@janiceilene janiceilene added Campus Experts + Docs program Issues for the Campus Experts + Docs program waiting for review Issue/PR is waiting for a writer's review pages Content related to GitHub Pages and removed triage Do not begin working on this issue until triaged by the team labels May 25, 2021
@janiceilene
Copy link
Contributor

Thanks for opening a PR and explaining the problem you're seeing with links in the updated content @AlexandraMMR 🎉 I'll triage this to be reviewed and one of the writers can help troubleshoot what's happening 💖

@ethanpalm ethanpalm self-assigned this May 26, 2021
ethanpalm
ethanpalm requested changes May 26, 2021
View reviewed changes
Copy link
Contributor

@ethanpalm ethanpalm left a comment

Choose a reason for hiding this comment

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

Hi @AlexandraMMR, thanks for opening this PR! It looks nearly ready to go ✨ There are a few small changes to make. Please look over the suggestions I made and let me know if any of them don't make sense. After they're incorporated, I will update this branch and get it merged 🎉

The deployment is looking good, however, there seems to be a bug with any pages that aren't under the /pages hierarchy, eg the Merge pull request link is under /github and seems that it can't access it 😞 Even happens when I do a search for the page so don't think it's an issue with the link, would appreciate it if you can have a look at it before approving. Thank you!

I'm not exactly sure why this is happening, but additional info from the content hierarchy is being added into the URLs, which is causing docs.github.com to say that the pages don't exist. The updated links I included should fix this 🤞🏼 and I'll double check them in staging after merging the changes.

data/reusables/files/choose_pull_request.md Outdated
@@ -0,0 +1 @@
1. Once you create the Pull Request with your changes, in the "Pull Requests" list, click the pull request you'd like to merge.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
1. Once you create the Pull Request with your changes, in the "Pull Requests" list, click the pull request you'd like to merge.
1. Create a pull request for your proposed changes.

Since the next action we want people to take is to create a pull request, I think we should be clear that they need to do that. So we can start this step by saying "create a pull request" and then the next step will be to merge the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ethanpalm Thank you so much for all the suggestions, I've applied them. I did add a second step after the Create a PR, to say they need to navigate to the PR list to merge as it wasn't obvious to the users in the intial thread this issue resulted from. Let me know what you think, if you think it's not needed we can remove it 😁

data/reusables/files/merge_pull_request.md Outdated Show resolved Hide resolved
data/reusables/files/delete_branch.md Outdated Show resolved Hide resolved
AlexandraMMR and others added 3 commits May 26, 2021 22:23
@AlexandraMMR
Update choose_pull_request.md
b14b508
@AlexandraMMR @ethanpalm
Update data/reusables/files/delete_branch.md
9878347 
Co-authored-by: Ethan Palm <56270045+ethanpalm@users.noreply.github.com>
@AlexandraMMR @ethanpalm
Update data/reusables/files/merge_pull_request.md
d99c14e 
Co-authored-by: Ethan Palm <56270045+ethanpalm@users.noreply.github.com>
@AlexandraMMR
Copy link
Contributor Author

@ethanpalm PS forgot to say I checked the deployment and the links are now working too, thank you! ✨

ethanpalm
ethanpalm approved these changes May 27, 2021
View reviewed changes
Copy link
Contributor

@ethanpalm ethanpalm left a comment

Choose a reason for hiding this comment

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

This looks ready to ship 🚀 I agree that including the second step to navigate to the correct PR can help people who might not know they need to take that step and it's really looking out for our users ✨

I'll update the branch and get it merged once all the checks are passing. The changes should be live within 24 hours 🎉

@ethanpalm ethanpalm merged commit 572e653 into github:main May 27, 2021
@github-actions
Copy link
Contributor

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're looking for your next contribution, check out our help wanted issues ⚡

@AlexandraMMR
Copy link
Contributor Author

This looks ready to ship 🚀 I agree that including the second step to navigate to the correct PR can help people who might not know they need to take that step and it's really looking out for our users ✨

I'll update the branch and get it merged once all the checks are passing. The changes should be live within 24 hours 🎉

Thank you! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ethanpalm ethanpalm ethanpalm approved these changes

Assignees

@ethanpalm ethanpalm

Labels
Campus Experts + Docs program Issues for the Campus Experts + Docs program pages Content related to GitHub Pages waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

"Adding a new page to your site" section missing critical last steps
3 participants
@AlexandraMMR @janiceilene @ethanpalm