Skip to content

Commit

Permalink
S ◾ Improvements to "write-a-good-pull-request" rule (#9835)
Browse files Browse the repository at this point in the history
* Improvements to "write-a-good-pull-request" rule

* Auto-fix Markdown files

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
tiagov8 and github-actions[bot] authored Feb 3, 2025
1 parent 22ec94e commit d551d7c
Showing 1 changed file with 35 additions and 37 deletions.
72 changes: 35 additions & 37 deletions rules/write-a-good-pull-request/rule.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,76 +36,74 @@ 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:

<!--endintro-->

### 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
Bad example - Pull Request title does not tell what issues have been fixed and how
:::

::: 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”
Expand All @@ -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
Expand All @@ -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).
Expand Down

0 comments on commit d551d7c

Please sign in to comment.