From d551d7c6cfa8251cac244ca8e01ab04a841527cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tiago=20Ara=C3=BAjo=20=5BSSW=5D?= Date: Mon, 3 Feb 2025 19:52:35 -0300 Subject: [PATCH] =?UTF-8?q?S=20=E2=97=BE=20Improvements=20to=20"write-a-go?= =?UTF-8?q?od-pull-request"=20rule=20(#9835)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Improvements to "write-a-good-pull-request" rule * Auto-fix Markdown files --------- Co-authored-by: github-actions[bot] --- rules/write-a-good-pull-request/rule.md | 72 ++++++++++++------------- 1 file changed, 35 insertions(+), 37 deletions(-) diff --git a/rules/write-a-good-pull-request/rule.md b/rules/write-a-good-pull-request/rule.md index 5b71c6eb34d..3e0938a4453 100644 --- a/rules/write-a-good-pull-request/rule.md +++ b/rules/write-a-good-pull-request/rule.md @@ -36,25 +36,23 @@ As a software developer, you are going to create Pull Requests (PRs) that you wa Including a little bit of context helps your reviewer understand changes quickly so they can review your PR faster and give better suggestions. -There are 2 easy things you can do to improve your Pull Request: +There are 2 essential things you should have when writing a Pull Request: -### 1. Write a concise and self-explanatory title +## 1. Concise and self-explanatory title -The key to writing a concise Pull Request is to base the PR itself on a PBI / issue. +Good **PR titles** provide a clear, concise summary of the change, helping reviewers quickly understand its purpose and priority. -Good **titles** cover: - -* What the Pull Request will do -* How the Pull Request achieved it +* What the PR will do +* How the PR achieves it * Emojis! Follow the [GitMoji.dev](https://gitmoji.dev) standard -Examples: +The key to writing a concise Pull Request is to base the PR itself on a PBI / Issue (assuming they are well written). +::: greybox **PBI title:** Product Backlog Item 100359: "Desktop App | Exporting occasionally failed" -::: greybox **Pull Request title:** Fix exporting ::: ::: bad @@ -62,50 +60,50 @@ Bad example - Pull Request title does not tell what issues have been fixed and h ::: ::: greybox +**PBI title:** Product Backlog Item 100359: "Desktop App | Exporting occasionally failed" + **Pull Request title:** 🐛 BUG - Fix desktop app exporting - prevent database concurrent access while exporting ::: ::: good Good example - Pull Request title briefly describes the fix that it has ::: -Having the **"What"** information allows the reviewers to quickly understand what this is about while having the **"How"** can help the reviewer to quickly understand how your PR solved the problem. Sometimes we might want to put the **"How"** in the PR body if it is too long or hard to explain in one sentence. +Having the **"What"** information allows the reviewers to quickly understand what this is about while having the **"How"** can help the reviewer to quickly understand how your PR solved the problem. Sometimes we might want to put the **"How"** in the PR descriptions (see below) if it is too long or hard to explain in one sentence. -### 2. Write a clear and concise description +## 2. Clear and informative description that gives all the context The Pull Request description is a medium for the developer to tell the reviewers what the changes are about. -::: info -**Tip:** For straight-forward changes the self-explanatory title might be enough. You should still include context so the reviewer knows what initiated the changes (examples below) -::: - -Good **descriptions** cover: - -* Context: - - * Relates to #{{ ISSUE NUMBER or URL}} (⚠️ see [avoid linking any Issues that you do not want to close](/avoid-auto-closing-issues/)) - * From email: {{ SUBJECT }} (like the rule [Warn then call](/warn-then-call)) - * As per my conversation with {{ NAME }} (like the rule [Do you send as per our conversation emails](/as-per-our-conversation-emails)) - * I noticed a problem: {{ DESCRIBE }} +Good **PR descriptions** provide background information helps others quickly understand the problem, solution, and rationale. This minimizes confusion, accelerates reviews, and improves overall code quality. ::: info -Linking the source to a PR serves as documentation on which development work that was done. It helps in the future to debug when and which changes were introduced and the original specification of that piece of work. +**Tip:** For rare straight-forward changes the self-explanatory title might be enough. You should still include context so the reviewer knows what initiated the changes (examples below) ::: -::: info -**Tip:** If you noticed that a change needed to be made and had no specific task/issue, use 'I noticed...' from above. -::: +Examples of sentences to have in a good PR description: -* Pair or mob programming? +* "Relates to **#{{ ISSUE NUMBER or URL }}**" (⚠️ see [avoid linking to Issues you do **not** want to close](/avoid-auto-closing-issues/)) +* "From email subject: **{{ EMAIL SUBJECT }}**" (similar to the rule on [warn then call](/warn-then-call)) +* "As per my conversation with **{{ NAME(S) }}**" (similar to "[as per our conversation](/as-per-our-conversation-emails)" rule) +* "Worked with **{{ @MENTION(S) }}**" (as per [pair or mob programming](/do-you-use-co-creation-patterns) rule) +* "This PR is to **{{ ACHIEVE THE FEATURE / FIX THE BUG / OTHER GOAL(S) }}**" (anything that was not possible to explain in the title) +* See **"{{ SCREENSHOT / DONE VIDEO }}** for more details" (to help the reviewer to understand the changes. E.g. Styling changes) -As per [Do you use Co-Creation Patterns?](/do-you-use-co-creation-patterns). E.g. "Worked with @bob, @mary and @jane" +If you noticed that a change needed to be made and had no specific task/issue, use: + +* "I/We noticed a problem: **{{ DETAILS }}**" -* What the PR is about and why you raised it +If there is an area you are uncertain about, add: -* How the PR will achieve the feature / fix the bug / other goals +* "I'm looking for feedback on this code" -* Screenshots / Done Videos to help the reviewer to understand the changes. E.g. Styling changes +If the PR is closing an email task after merged, remember to refer back to it: -* Tell reviewers if there is an area you are uncertain about. E.g. "I'm looking for feedback on this code" +* "Done - see email: **{{ EMAIL SUBJECT }}** + +::: info +Linking the source to a PR serves as documentation on which development work that was done. It helps in the future to debug when and which changes were introduced and the original specification of that piece of work. +::: ::: greybox **PR title:** Update Rule “meaningful-pbi-titles/rule” @@ -129,7 +127,7 @@ Figure: OK example - Clear and concise description, however it's not clear what **PR title:** Update Rule “meaningful-pbi-titles/rule” **PR description:** -From email, subject: SSW.Rules - Video caption missing +From email subject by @bob: **SSW.Rules - Video caption missing** Added missing video caption + removed unnecessary brackets ::: ::: good @@ -150,16 +148,16 @@ There is also well-known Pull Request semantics like [Conventional Commits](http **Q: Are you making many small changes?** -A: You should summarize by saying: *“Improved readability”* OR *“Fixed typos and grammar”*. +**A:** You should summarize by saying: *“Improved readability”* OR *“Fixed typos and grammar”*. **Q: Are the changes big and complex?** -A: You should include a demonstration of the change. +**A:** You should include a demonstration of the change. E.g. A [screenshot](/screenshots-avoid-walls-of-text) to show text/UI changes, or a [Done video](/record-a-quick-and-dirty-done-video) to demo functionality changes. **Q: Are you using a CMS editor (like Netlify or Tina)** -A: CMS editors may automatically add a placeholder description. If you're using a CMS to make your changes, you may need to go to the PR afterward and update the description to include the context. +**A:** CMS editors may automatically add a placeholder description. If you're using a CMS to make your changes, you may need to go to the PR afterward and update the description to include the context. ::: info **Tip:** Ensure devs follow these tips by using a template. [Learn more and check out a template example](/use-pull-request-templates-to-communicate-expectations).