Pull Requests In A Team Environment

Kirsty-lee Walker
DVT Software Engineering
6 min readOct 4, 2022

Dear readers. You will have come across Pull Requests (PRs) if you come from a technical background. Most people dread PRs, but it is probably because teams do it incorrectly or are too critical when reviewing PRs.

What Is A Pull Request?

A PR is a mechanism for a developer to notify team members that they have completed a feature and are requesting the team to review their code. Once the reviewers have made comments, requested changes, or approved, the feature is merged into a bigger code base and eventually released to the consumers.

Photo by Goran Ivos on Unsplash
  • Quality assurance practice — Developers believe and strive to meet all 7 criteria.
  1. The code is readable
  2. The code is scalable
  3. The code is testable
  4. The code solves the problem at hand
  5. The code fails gracefully
  6. The code is easy to extend
  7. The code is reusable

In reality, developers rarely meet all 7. Sometimes areas of improvement are identified when other team members review their code. Therefore it is the entire team’s responsibility to ensure that any code that goes into the bigger codebase meets a certain standard and makes it better than previous versions, even if the code is not perfect.

  • Collaboration session to share knowledge —PRs are an opportunity to share knowledge between the team. There are many different ways to solve a problem. By using a PR, others can see the thought process of a task, which can alter how the team solves problems in the future. Similarly, PRs offer an excellent way for senior team members to pass best practices to less experienced developers.
  • Pull Requests are a documentation tool — In future use cases, descriptive PR comments and descriptions give better insight into the developer’s and team’s thought processes and act as a traceability tool. For junior team members, it is a way to become more familiar with the codebase by going through previous PRs.
  • Not a testing environment —PRs are not supposed to test people’s knowledge or loyalty to the product. A PR is about growing together and ensuring that code goes out that meets specific standards, solves the problem at hand and improves the overall quality of the product.
Photo by Mark Duffel on Unsplash

Suggestions When Creating A Pull Request

PRs are very expensive to do because it doubles the development time as someone has to stop their work and read through the code. Below are suggestions to avoid wasting the other team members’ time.

5 suggestions before creating a Pull Request

  1. Review your own code
  2. Look for typos
  3. Remove logging or unused code
  4. Ensure code is readable and debuggable
  5. Look for mistakes in logic

Four Common Pull Request Scenarios

  • The chosen one — When large amounts of files are changed, the PR usually goes straight through without any comments or is never reviewed.
  • The Boomerang — Once a PR is submitted, it always returns no matter how many times it changes.
  • The Monster —Once a PR is added to the large code base, it is not in sync, and there are many conflicts.
  • The black hole — Once the code is submitted to a PR, it is never seen again.

Below are suggestions on how to avoid the four above scenarios from happening.

Suggestions When submitting A Pull Request

  1. Make sure that your PRs are small (The Chosen One) — If PRs are too large, break the PR down into smaller PRs that are easier to review.
  2. Give context on why the PR is needed (The Boomerang) — Providing relevant context about the PR makes it easier for the team to review as they know what to look for and understand why you did certain changes. This allows code to pass quickly through the PR phase, testing, and to the consumers.
  3. Keep code up to date (The Monster) — Ensure that your code is updated with the most recent version to ensure no conflicts when the code goes into the larger code base.
  4. Only one ticket per Pull Request (The Black Hole) — Only focus on the task in the ticket; if you have multiple tasks in a single PR, it makes it difficult to see why a specific change was made and makes PR’s a lot slower to review.

Mindset when Reviewing Pull Requests

Regardless of what we discover, we understand and truly believe that everyone did the best job they could, given what they knew at the time, their skills and abilities, the resources available, and the situation at hand. — Norm Kerth

Suggestions When Reviewing A Pull Request

  • Be polite — With the above quote in mind, it is important to remember that everyone in the team is doing their best. PRs are meant to be a learning experience to see other team members thought processes. If PRs become too personal, they will lower productivity and make it a very toxic environment to work in. Therefore when reviewing PRs, all team members should be respectful.
  • Set up a dedicated session for reviewing PRs — Team members get very busy, so it is important to set up a dedicated time to review PRs. Doing this ensures that the team can fully focus on reviewing the PR. This can lead to more constructive feedback through comments and lead to better code going into the larger code base.
  • Be careful of being a perfectionist - It is not always possible to be perfect. When there are many comments from a single reviewer, it is best to rethink if you can rather add follow-up tasks and approve the current PR.
  • Do a full review, do not stop at the first commit — It is not helpful for a reviewer to stop at the first comment and then stop their PR as this wastes time and causes a lot of back and forth. When reviewing, go through the full PR and make all your comments.
  • Be concise in your comments and provide suggestions - Language is key. If a reviewer is unhappy with something, give an example and explain why instead of just saying it is incorrect. Also, remember that instead of saying “this is bad”, say “A suggestion would be…”.
  • Do not communicate only through PRs — As a reviewer, if something is unclear, it is better to call the reviewer instead of only communicating via PRs as this communication becomes stale.

Suggestions When Changing A Pull Request

  • Value team members' feedback — Read the comments carefully and see if there is any merit to what the reviewers say. If there is merit, make the necessary changes. If there is no merit, explain why you implemented it that way in a comment. Lastly, if you do not understand the statement, ask the reviewer for more clarification.
  • Communication is key for effective PRs — If a reviewer's comment is unclear, talk to them over the phone to understand their point of view.

Conclusion

Reviewing another team member’s code is a difficult task. PRs are not only a commitment to the team but also a commitment to the product. The PR process must be as easy as possible because PRs take time and energy. Following the guidelines above provides an excellent opportunity for team members to exchange knowledge, identify and correct issues early on, and learn and grow together from the PR experience. The result ensures that stakeholders get features released quickly to the consumer.

--

--

Kirsty-lee Walker
DVT Software Engineering

I am passionate about development and solving problems in unconventional ways. I specialise in IOS development specifically Swift, SwiftUI, Combine and UIKit