Guidelines for the author(s)
Open a good pull request
Title Structure
Standard
The title (name) of the pull request must contain both the user story ID and the story title.
[<User Story ID>] <User Story Title>
For example:
-
The following title contains a user story ID and a story title from Shortcut:
[sc-32874] [UI] As a user, I can access my home screen
-
The following title contains a user story ID and a story title from Jira:
[ORANGE-1920] [Integrate] As a user, I can access my home screen
-
The following title contains a user story ID and a story title from GitHub:
[#123] [Backend] As a user, I can access my home screen
Skip CI
Currently, only Bitrise supports skipping builds via the pull request’s title. To not waste CI/CD resources on Bitrise, in specific situations, the developer can skip the build if it is expected to fail or be re-triggered again by adding a [skip ci]
prefix in the pull request’s title. Then to resume the workflow trigger, remove the [skip ci]
prefix.
[skip ci] [sc-32874] [UI] As a user, I can access my home screen
# or
[skip ci] [#123] [Backend] As a user, I can access my home screen
Description
The description of the pull request must abide by this template.
The first line of a pull request description should be a link to the user story to make it easier for reviewers to get the complete requirement.
For example:
https://app.shortcut.com/nimble-website/story/23792
In terms of the body, there are three main sections for the author of the pull request to fill in:
-
What happened
Provide a description of what changes are being made. It should be informative enough that other developers do not have to read the pull request code or its whole description to understand what the pull request did.
-
Insight
This section should fill in the details, guidance, and additional information for reviewers to understand the changes.
For example, it can be used to inform about:
- A description of the issue that is being solved.
- Why the current approach is the best? Are there any links to the documents? Are there any shortcomings to the approach?
- How can the code reviewers test the pull request locally or remotely?
-
Proof of Work
Provide any visuals that can give assurance that the changes are safe to merge.
Here is an example of a good pull request:
Add Reviewers
- At least two reviewers must be added to each pull request: either two team members in the same squad or, for a smaller project, one team member in the project and one “outsider” (not working in the project). The “outsider” must be defined as a reviewer member of the squad in the team organization diagram.
- Among the reviewers must be the Team Lead and one other developer.
Add Metadata
In order to filter pull requests by user story types, and priorities, it’s recommended to:
- Assign the appropriate labels to the pull request. Labels are used to categorize the type of the pull request (
feature
,chore
, orbug
) and its priority (high
,normal
, orlow
). - Assign a milestone to the pull request. Usually, a milestone corresponds to a release version.
The size of the pull request
The scope pull request must be limited to a single story:
- The pull request makes a minimal change that addresses just one story. Based on the areas of implementation, the author should create a pull request that implements only one area.
- The pull request must also include tests for the changes made.
- Extract some changes and refactorings into future stories if the requested changes are large and/or out of the initial scope of the pull request.
Advantages of one-story-scoped pull requests
-
A small pull request is reviewed faster i.e., the code reviewer spends less time reviewing each pull request on average.
-
Code reviewers can easily just focus on one thing (one story) at the moment i.e., a single list of acceptance criteria from one story helps the code review spends less effort to check and validate.
-
Code reviewers and the author can easily and quickly detect bugs i.e., code reviewers and the author can easily assess a small pull request. As the result, they can think more about edge cases, issues, or bugs.
-
The Team Lead can merge pull requests quickly: When working with a large pull request, normally, it will take a long time, and it will have lots of conflicts after rebasing compared to a small pull request.
Optimal Line of Code Count (LOC)
Regarding the pull request size, an efficient pull request size is below 300 LOCs on average. In case a one-story-related pull request might still involve many files and become overwhelming to review, it is recommended to split that into smaller ones. This enables earlier reviews and increases the focus for the reviewers.
Open pull requests early on
- Do NOT wait to complete all requirements before opening a pull request:
- On GitHub and GitLab, open pull requests in draft mode since both version control providers have built-in support for this.
- On Bitbucket or any provider not offering draft pull requests, add the prefix
WIP
to the pull request title (e.g.WIP - [<User Story ID>] As a user, I can search for articles
).
- Draft or Work-In-Progress (WIP) pull requests are encouraged to both get early feedback on implementation and share it with the team. Indeed, for efficiency purposes, the Team Lead can detect issues early in the implementation choices hence rerouting efforts earlier in the sprint. Besides, other team members can cherry-pick code from the branch even though it is not complete.
Best practices
In order to reduce the pull request pickup time, the following best practices are recommended for the author:
-
Share the implementation details with the team before implementing. The code reviewers and the author will get through code review process speedier and more efficient (decrease the code review time, and have fewer comments).
-
Check open pull requests before implementing a new story. Prioritize open pull requests by fixing or replying to all the comments as soon as possible to improve the code review time.
Collaborate with code reviewers efficiently
Notify code reviewers
A pre-requisite before notifying code reviewers, DO a self-code review before making the pull request ready for review. As mentioned above, it is encouraged to create a draft or WIP pull request, the author should do a self-code review to prevent code convention mistakes, typos, or to validate their overall solution again.
Based on the Review Time section, an efficient review time is between 1 to 2 days on average, it is important to follow the pull request’s progress. Here are some best practices to notify code reviewers and improve the review time:
-
GitHub, GitLab, or Bitbucket will automatically notify the code reviewers about the pull request, the author of the pull request does NOT need to ask for code reviewing immediately after creating the pull request.
-
If there are no activities on the pull request after a half-day, it is recommended to mention all code reviewers on the Slack dev channel to make sure all code reviewers already planned for code review.
-
The author of the pull request can request the Team Lead to add priority labels for the pull request, all code reviewers can know when to review and follow by the order when having more than one pull request.
-
In case there is a code reviewer who requested changes on the pull request, do not forget to
re-request review
after fixing or replying to the code reviewer’s comments. -
In terms of urgent features or issues, do not hesitate to send a message directly to the code reviewers.
Handle comments of code reviewers
-
Seek to understand the reviewer’s perspective. Always ask yourself “What is the constructive thing that the reviewer is trying to communicate to me?”. Assume the best intention from the reviewer’s comments.
-
Be grateful for the reviewer’s suggestions (e.g.
Good catch. I will make that change.
). Never respond in anger to code review comments. -
Prefer to respond to every comment by replying with a text message. Avoid only adding an emoji reaction to the review comment unless 1) a similar comment was already addressed with a proper response or 2) the review comment is minor e.g. suggesting a text replacement, pointing out a typo, etc.
-
Push commits based on rounds of feedback as isolated commits to the branch. As a good practice, reply to the comment with a link to fix (e.g.
Fixed in a4994ec
). -
Use tools (e.g. Email, LinearB bot, or Slack bot) to get notifications from pull requests.
-
In case the author of the pull request does not agree with the code reviewer, the author needs to understand what the reviewer is asking for, and ask for clarification if needed. If the author and code reviewer still do not have the same voice, then bring the discussion to the project channel or ask the Team Lead for the final decision.
-
The code reviewer would be the one closing the comments most of the time, however, the author can mark a review comment as resolved when:
- The solution is clearly suggested/written in the initial comment — i.e. using
change suggestion
. - When the comment mentions specific typos.
- When the request for change is evident to the author and it was just a miss-out, e.g. basic conventions, code formatting, forgetting to run linter, removing a file from code versioning, etc.
- The solution is clearly suggested/written in the initial comment — i.e. using