Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding new rule Do you still review Pull Requests when you are not a required viewer? #6922

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ index:
- avoid-auto-closing-issues
- use-squash-and-merge-for-open-source-projects
- standard-set-of-pull-request-workflows
- review-prs-when-not-required
---

Pull Requests are the backbone of an effective development team. That's why it's crucial to ensure that everyone on the team understands the expectations around Pull Requests.
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
---
type: rule
title: Do you still review Pull Requests when you are not a required viewer?
guid: 2e3f7d09-8004-4de5-a393-b8341e123534
uri: review-prs-when-not-required
created: 2023-10-6T00:00:00.0000000Z
authors:
- title: Gordon Beeming
url: https://ssw.com.au/people/gordon-beeming
- title: Matt Goldman
url: https://ssw.com.au/people/matt-goldman
related:
- use-branch-protection
- checked-by-xxx
redirects: []

---

As a repository maintainer you would generally setup a branch policy which may include a certain number of reviewers before a pull request can be completed, this could also include that certain reviewers are required instead of this being wide open to just anybody. This is a great way to ensure that code is reviewed before it is merged into the main branch.

However, this does not mean that you should not review pull requests that you are not required to review.


<!--endintro-->

Let's take a look at some of the benefits of reviewing pull requests, even if you are not a required reviewer.

As a junior developer, you may not feel comfortable reviewing a senior developer's code, but you should. You may not be able to provide any feedback around standards and best practices, but you can ask questions. This is a great way to learn and understand why the senior developer did something a certain way.

When you ask questions in a pull request, you are giving the author the opportunity to take a step back and think about what they have done and how they've done it, explain their thought process and potentially where you could learn more about the approach. Alternatively, you could point out a different way of achieving the same result which could be a better approach and they could change their PR.

- ✅ You could learn something new
- ✅ The author could learn something new
- ✅ Congrats... you've just created some micro-documentation that may never have existed before

The more people have approved a pull request, the more confidence you can have that more people in the team have looked at and agree that the code should be merged into the branch. This is effectively the same approach as getting a "Checked by xxx" on an email, see [Do you use 'Checked by xxx'?](https://www.ssw.com.au/rules/checked-by-xxx/).

- ✅ Shows the team you understand and agree with the code changes
Loading