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.
Everybody strives to be perfect, but mistakes are normal, and it is easy for a developer to skim over errors when they've read their own code code hundreds of times!
Pull requests are the best way to ensure that two sets of eyes see every change - so the responsibility is never solely on the person writing the code.
As a software developer, you are going to create Pull Requests (PRs) that you want to be easy for others to review and approve. The quality of a Pull Request can vary - from cryptic to well-written.
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 essential things you should have when writing a Pull Request:
The use of Draft Pull Requests is a handy practice to indicate work in progress promoting early collaboration and continuous feedback. This approach enhances code quality, reduces duplication, and helps to maintain a transparent and efficient development pipeline. Creating Draft Pull Requests will also trigger GitHub Workflows so developers get early feedback on the quality of their changes.
Draft Pull Requests are less effective if they are not clearly marked as Draft, as is the case on GitHub. To make them clearer, use a naming convention like
🚧 WIP: {{PR Title}}to clearly show that it is a Draft Pull Request.GitHub
❌ Figure: Bad example - The default experience lacks clear indication that this is Draft Pull Request
✅ Figure: Good example - Add prefix with 🚧 emoji to clearly indicate it is a Draft Pull Request
If you want to go one step further, you can add the WIP App to your repo. The WIP App prevents the merging of Pull Requests with "WIP" in their title. When you are ready to go, you just remove the WIP prefix.
✅ Figure: Good example - WIP app catching Draft Pull Request
Azure DevOps
✅ Figure: Good example - Clear naming and indication of a draft pull request
The PR author is responsible to follow up and get PRs merged as soon as possible. An "over-the-shoulder" review is one of the best ways to avoid merge debt.
When a pull request (PR) is ready to be reviewed, get someone with you either in-person or on call, and go through the PR together. This not only allows you to demo the content of the PR but also talk with the person taking feedback when needed.
When a pull request is left open for a long time, it becomes stale and accumulates merge debt. The longer it remains open, the more changes occur on the main branch, increasing the work needed to update and merge the PR. This can lead to conflicts, bugs, and unhappy developers.
Pull Request templates are a great way to communicate expectations to developers. You should create different PR templates for different types of PRs. For example, you can have a PR template for bug fixes, a PR template for new features, and a PR template for refactoring. You are also able to create specific PR templates for specific code paths.
For the original article check out Don't "Push" Your Pull Requests by Ilya Grigorik.
Open source software projects love it when the community get involved and pitch in.
It's great when someone fixes a bug.
A short PR to fix a small problem is easy to review and accept.
Before merging any Pull Request (PR) that might affect your application's behavior or performance, you should run comparison tests between the production environment and the PR deployment slot to identify potential issues.
Pull requests are a fundamental feature of collaborative software development, allowing contributors to propose changes to a project and receive feedback from other developers. When reviewing a pull request, it can be tempting to make additional changes beyond those requested by the PR creator.
Certain projects (E.g. SSW.Rules) allow these additional edits, without the need for extra reviews by someone else. Knowing that, it's crucial to make sure the changes are correct, necessary and beneficial to the project before adding them.
When the Product Owner makes a verbal change request or decision, you might consider sending an email and CCing others, but that limits visibility to only those on the thread.
A better approach is to update the PBI to document the change, making the conversation visible to the entire team. Use comments with tags/mentions (@username) to notify the Product Owner and any relevant team members. This ensures they receive email notifications about the update.
GitHub provides a way to link issues to PRs. This is useful to see what PRs are associated with what issues. However, when you make this link, the issue will close when the PR is merged.
This is not a good idea because it can cause Issues to be closed prematurely. This can lead to confusion and lost work.
Luckily, GitHub provides a way to avoid this auto-closing behavior by disabling it in the repository settings.
When you are working on an open source project, you will often get pull requests from contributors. When you merge these pull requests, you should use the "Squash and merge" option. This will squash all the commits into one commit and then merge it into the target branch. This is a good practice because it keeps the commit history clean and easy to read. It also makes it easier for other developers to understand what changes were made in each PR.
Getting started with a new repository can be daunting, especially if you are new to the project. Having a standard set of pull request workflows can help you get started and make sure you are following the same process as everyone else on the team.
A few standard workflows helps developers see a consistent process across all repositories. This makes it easier for developers to get started with a new repository and makes it easier for developers to move between repositories as the feedback they get from each pull request is consistent.
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.
These days Pull Requests are the de facto standard for getting code reviewed. Once a developer has finished their change, they will typically submit a Pull Request and move on to their next task. This allows for an asynchronous process to take place which may seem like a good idea, but often is not and can also lead to inefficiencies.
When using co-creation patterns such as pair-programming or mob-programming, you should make sure that all the developers get attribution. When done correctly co-authored commits stand out as a testament to teamwork and shared responsibility, reflecting the collaborative efforts and diverse expertise contributed to each change.
Figure: GitHub - Co-Authored Commit
Managing webpages can be challenging, especially in projects with many contributors. When editing a page, a common problem is not knowing who the original author was. This is bad, because it's important not to change something that was there for a reason!
Maintaining a clean git history is important for readability and understanding the changes that have been made to a codebase. This is especially important when working in a team, as it allows you to see the changes that have been made to a file over time, and who made them. This can be useful for debugging, and for understanding why a particular change was made.
Things that create a good git history include:
Normally, the best way to provide feedback on content changes is to use the change X to Y format. When it comes to reviewing Pull Requests (PRs) in GitHub, this is not the case - it's inefficient for the PR owner. They have to manually interpret each suggested change, implement them in the code, and then commit the changes, which can be time-consuming.
When reviewing another developer's pull request, review comments can fall into a mix of categories. Some changes might be required, while others might be optional. Do you know the best way to tell the requester which is which?