-
Notifications
You must be signed in to change notification settings - Fork 59.8k
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
Conversation
🙌 You're on a roll @codewithdev 🎉 I'll get this triaged for review ⚡ |
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. |
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 You can do this in the UI here: https://github.com/codewithdev/docs |
@felicitymay Fortunately, my main branch is |
@codewithdev - thanks for updating the I've pushed a commit that merges the changes from
I'll take a look at your changes soon. |
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.
@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.
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>
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>
@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. Let me know if there need some certain changes in the support community reusable. |
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 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 😄 |
There needs a little update on the reusable |
Waiting for the Final Review 👀 |
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. |
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 👍🏻 |
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'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 ✨
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 ⚡ |
Learned many things out from this PR. Thanks, @felicitymay for the prime suggestions and reviews. ✨ |
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. ✨ |
Why:
Closes #4849
What's being changed:
Updated the footers in Quickstart map topics,
Check off the following:
Writer impact (This section is for GitHub staff members only):