From 6d59ebba31f511c56b40333285fbc23eb378e937 Mon Sep 17 00:00:00 2001 From: Huw Diprose Date: Tue, 30 Jul 2024 15:17:37 +0100 Subject: [PATCH 1/2] Chore: Change apostrophe within review content to U+0027 I believe this is in line with tech writer style guide, it certainly caused my editor to get confused about what was code and where pigmenting should start and stop. No harm it seems to me in using a quote mark that is unambiguously content and not code. --- source/standards/pull-requests.html.md.erb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/source/standards/pull-requests.html.md.erb b/source/standards/pull-requests.html.md.erb index 0cc331fa..a1066c92 100644 --- a/source/standards/pull-requests.html.md.erb +++ b/source/standards/pull-requests.html.md.erb @@ -5,7 +5,7 @@ review_in: 12 months --- # <%= current_page.data.title %> -Pull requests (PRs) let you tell others about changes you've pushed to a branch in a repository on GitHub. +Pull requests (PRs) let you tell others about changes you’ve pushed to a branch in a repository on GitHub. ## Why should you use pull requests? @@ -46,11 +46,11 @@ As such, the canonical description of changes should be in the commit messages. ### Reviewing a request -Taking time to properly review a PR is as important as making the change itself, and it is crucial that we show compassion when critiquing someone else's work. +Taking time to properly review a PR is as important as making the change itself, and it is crucial that we show compassion when critiquing someone else’s work. April Wensel has a talk about [Compassionate-Yet-Candid Code Reviews](https://www.slideshare.net/AprilWensel/compassionate-yet-candid-code-reviews). -It suggests 3 key questions to ask ourselves when reviewing or commenting on someone else's work: +It suggests 3 key questions to ask ourselves when reviewing or commenting on someone else’s work: - Is it true? - Is it necessary? @@ -116,7 +116,7 @@ If we do not want to merge it because it does not fit with our plans, thank them If it fits, but we cannot merge it due to quality or style issues, then tell them that we are able to merge the PR if they make some changes. -We can tell the requester what improvements we'd like to see, but we should not require the contributor to make them all. For example, we might add missing tests ourselves or collaborate with them to add tests. +We can tell the requester what improvements we’d like to see, but we should not require the contributor to make them all. For example, we might add missing tests ourselves or collaborate with them to add tests. We should close PRs due to lack of activity but invite people to reopen them if they pick things up again. From c15cc8735dd545179070d1bdd9b202f813f91d65 Mon Sep 17 00:00:00 2001 From: Huw Diprose Date: Tue, 30 Jul 2024 15:52:35 +0100 Subject: [PATCH 2/2] Minor revision to examples of sources for commit messages Our ticketing and support systems are becoming more complicated across GDS, hopefully this is a minor clarification that avoids a double take from folks not using zendesk --- source/standards/pull-requests.html.md.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/source/standards/pull-requests.html.md.erb b/source/standards/pull-requests.html.md.erb index a1066c92..d61f40c5 100644 --- a/source/standards/pull-requests.html.md.erb +++ b/source/standards/pull-requests.html.md.erb @@ -35,7 +35,7 @@ You should make sure that: - your local repository has the most recent changes on the target branch - the title and description of the PR are clear and accurately represent your changes -- the title and description reference the source of the change, which could be a Zendesk ticket or a Jira card +- the title and description reference the source of the change, which could be a support ticket (Zendesk, or Service Now) or a Jira card - the description contains links to any related PRs - any screenshots have text descriptions for accessibility - any contentious changes or side effects have clear descriptions of the pros and cons