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.

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

Updated Quickstart Footers #7392

Merged
merged 25 commits into from
Jun 22, 2021
Merged

Updated Quickstart Footers #7392

merged 25 commits into from
Jun 22, 2021

Conversation

codewithdev
Copy link
Contributor

Why:

Closes #4849

What's being changed:

Updated the footers in Quickstart map topics,

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 Jun 15, 2021
@janiceilene
Copy link
Contributor

🙌 You're on a roll @codewithdev 🎉 I'll get this triaged for review ⚡

@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 and removed triage Do not begin working on this issue until triaged by the team labels Jun 15, 2021
@felicitymay
Copy link
Contributor

Hi @codewithdev - an internal project moved these articles to a new location, which caused these conflicts. I'll checkout your branch locally and see if I can resolve them.

@felicitymay
Copy link
Contributor

Hi @codewithdev - I haven't worked with forks for a while and had forgotten that it would be slightly more complicated to get the changes from main. Would you be able to update the main branch of your fork from the docs repository?

You can do this in the UI here: https://github.com/codewithdev/docs

image

@codewithdev
Copy link
Contributor Author

codewithdev commented Jun 22, 2021

@felicitymay Fortunately, my main branch is even with the upstream repository.

@felicitymay
Copy link
Contributor

@codewithdev - thanks for updating the main branch for your fork 🙇🏻‍♀️

I've pushed a commit that merges the changes from main into your branch as follows:

  1. Made a local clone of your repository.
  2. Updated my local copy of your main branch with the latest changes.
  3. Checked out your branch.
  4. Merged main into your branch.
  5. I was expecting to need to resolve some conflicts, but Git was able to resolve the changes from main without help.
  6. Pushed the resulting merge commit to your branch on GitHub.

I'll take a look at your changes soon.

Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

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

@codewithdev - thank you so much for taking the time to think about what a new user needs to know when they finish working through each of these articles. Your proposals will help them decide what to do next 💖

I've added a lot of comments, but most of them are just for small changes. Since you're an active contributor, I've taken the time to add some explanations of most of the suggested changes.

For smaller pull requests, we normally commit the changes immediately to speed up the turn around time for merging the work. Since this is a larger pull request and you've put a lot more thought and work into it, I'm going to leave you to take a look at the suggestions and commit them. If you have questions about any of them, let me know.

Also, let me know if you have any questions about the suggestion to update the {% data reusables.support.connect-in-the-forum-bootcamp %} reusable.

content/get-started/quickstart/set-up-git.md Outdated Show resolved Hide resolved
content/get-started/quickstart/set-up-git.md Outdated Show resolved Hide resolved
content/get-started/quickstart/set-up-git.md Outdated Show resolved Hide resolved
content/get-started/quickstart/create-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/be-social.md Outdated Show resolved Hide resolved
content/get-started/quickstart/create-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/fork-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/fork-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/fork-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/fork-a-repo.md Outdated Show resolved Hide resolved
codewithdev and others added 3 commits June 22, 2021 18:07
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
@codewithdev codewithdev dismissed a stale review via bc33de4 June 22, 2021 13:03
codewithdev and others added 8 commits June 22, 2021 18:36
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
Co-authored-by: Felicity Chapman <felicitymay@github.com>
@codewithdev
Copy link
Contributor Author

@felicitymay I've updated the internal docs where reusable (connect-in-the-forum-bootcamp.md) is located.

Since, this reusable is used only on these four articles thus I've applied all the changes that you've mentioned in the comment.
Also I've updated the necessary changes that you've suggested and replaced the community support with just, {% data reusables.support.connect-in-the-forum-bootcamp %}

Let me know if there need some certain changes in the support community reusable.

@felicitymay
Copy link
Contributor

Thanks for making all those changes. 💖

I'm not sure why the deployment to staging is still showing as pending, I'll update your branch with changes from main which should retrigger the deployment. 🤞🏻

Once we can see the articles on staging, we can have a final check for anything that's slipped through the review, and then merge 😄

@codewithdev
Copy link
Contributor Author

codewithdev commented Jun 22, 2021

There needs a little update on the reusable (docs/data/reusables/support/connect-in-the-forum-bootcamp.md) and we can remove that extra ". dot"

@codewithdev
Copy link
Contributor Author

Waiting for the Final Review 👀

@felicitymay
Copy link
Contributor

There needs a little update on the reusable (docs/data/reusables/support/connect-in-the-forum-bootcamp.md) and we can remove that extra ". dot"

That's a great point - well spotted ✨

In this case, it would be better to keep the "dot" (period) in the reusable and delete it from the articles. We try to keep reusables as full sentences that can be reused easily. For more information, see Reusables and variables.

I've spotted a couple more tiny things that I missed previously - they're all so small, it's probably quicker for me to add suggestions for them, commit them all, and then your pull request will be ready to merge.

@felicitymay
Copy link
Contributor

I should have said - apart from the very small things I spotted, this is now looking great. Congratulations on making such good use of the reusable too 👍🏻

felicitymay
felicitymay previously approved these changes Jun 22, 2021
Copy link
Contributor

@felicitymay felicitymay left a comment

Choose a reason for hiding this comment

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

I'm going to move the "dot" (period) back into the reusable and add a missing "the" but otherwise this looks great and will be ready to merge with these very minor adjustments.

Thanks for your patience with this process ✨

content/get-started/quickstart/create-a-repo.md Outdated Show resolved Hide resolved
data/reusables/support/connect-in-the-forum-bootcamp.md Outdated Show resolved Hide resolved
content/get-started/quickstart/set-up-git.md Outdated Show resolved Hide resolved
content/get-started/quickstart/fork-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/create-a-repo.md Outdated Show resolved Hide resolved
content/get-started/quickstart/be-social.md Outdated Show resolved Hide resolved
@felicitymay felicitymay merged commit caaea52 into github:main Jun 22, 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

@codewithdev codewithdev deleted the codewithdev-docs-2 branch June 22, 2021 17:15
@codewithdev
Copy link
Contributor Author

Learned many things out from this PR. Thanks, @felicitymay for the prime suggestions and reviews. ✨

@felicitymay
Copy link
Contributor

Learned many things out from this PR.

I'm glad it was useful. It's difficult for a reviewer to get the balance right. We don't want to make the process too slow so we often just make changes, but you seemed to want to know more. ✨

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Campus Experts + Docs program Issues for the Campus Experts + Docs program waiting for review Issue/PR is waiting for a writer's review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update quickstart footers in getting started category
4 participants