Crafting The Ideal Merge Request
Think of Merge Requests as a nicely wrapped gift to your peers. It should make them happy and not annoyed.
Merge Requests are your critical methodology of working well as a group in software projects. They contain everything that is needed in order to push a software project forward and if you want to have a successful project you need a robust and streamlined process. You will make thousands of Merge Requests over your lifespan and getting the process right for yourself will save you time and frustration along with your teammates.
The merge process will contain your code which is the most critical asset, however it is not the most critical point of get code merged oddly enough. Why? Merge Requests are social signaling via knowledge sharing. It’s someone saying I made this code, and I’d like to combine it with the existing code, and here’s how I did it. You are putting your code into the mercy of the reviewers of the code repository and only with their approval may you get in. Viewing it in this lens should change how you feel about the process now. It’s not just about the code, it’s about the entire presentation of your merge request. If your presentation is terrible and unclear you will get rejected for a variety of reasons. Even if your code is perfect from a technical standpoint many things can stall or even sink your merge. This could be a failure to explain the changes, not filling out required fields in the template, no links to a ticket with business justification, an inability follow the code conventions, or just an inability to format the code correctly. I often have people submit MRs to my projects and they just ignore the entire template, don’t link tickets, and plenty of other issues. This requires me to take time out of my day to repeat things they are unfamiliar with and hand hold them to get their code in. We can streamline this process so that we spend less time reviewing MRs and also make the burden less on the committer.
In this article we will discuss what are necessary components for a Merge Request, examples of a good one, and some tips for creating merge templates so that your team can follow. I will also discuss some good practices to set your repo up in Gitlab so that you can incorporate these changes into your development lifecycle. By applying all these techniques we can save everyone on the team more time by getting code merged faster and allowing you to finishing your work faster.
Basic of a Merge Request
The structure of your merge request is going to heavily depend on the process put in place by your org. The most standard ones days are based around Github and Gitlab styles, but there are also older styles done via Gerrit or TFS. I’m going to focus on the former since those are much more popular and specifically use Gitlab as my demo vehicle. These use Markdown as the syntax which enables a rich document to share with your team. Most well established companies will have existing established conventions to follow and if you don’t follow them you will be denied or stuck in MR purgatory. You can see Google’s process here for some inspiration at the Enterprise level. Your hand will be held in this process, so just follow everything the team asks of you as best as you can and you will not have an issue as long as the code is good.
Many smaller companies or open source projects keep it much looser as a process and that is where we will try to focus on for this. Know that the Merge Request process should seek to provide extreme clarity to the reviewer, share knowledge out, and also make the product better, so focus on that when creating the request. Your MR will be an all encompassing technical document that contains all the following information for the reviewer to make an informed approval.
An overall description of the work
This tells the high level story of your work. Do not get too technical here as the code should primarily speak for itself if you follow good coding conventions like “Clean Code”. I prefer to leave my own comments on the MR itself to guide the reviewer towards clarity as that keeps the context local and therefore easier to understand. Here are some things to focus on in this section:
It will note critical ideas of the story to look for
EX: “Changing Database access to use environment based credentials instead of hard coded ones”
EX: “Update the styling on the main menu to not overflow on narrow screens”
If a non trivial change, a link to your ticketing system with the business requirements
Links to any design documents you made. If you have a complex change set including diagrams and low level designs are a must
Action items other team members must do, due to a new process, breaking changes, or testing requirements
Links to related MRs and also reference documents used, such as MDN docs, language docs, and other key reference points or blogs
Proof of working functionality
Usually this is a before and after picture or a live site for users to look at and confirm with their own eyes. I’ve been burned before by something being “fixed” but it actually never was. Trust, but verify always.
For UI work with would be screenshots or a video showing the fix embedded into the MR doc.
For non visual based work, this could be many things:
API work: screenshots of an HTTP request/response showing it works from Postman or the Browser, or .HARs of the process, or JSON responses
CI/CD work: A link to a passing pipeline with the fix or new stage
Infrastructure work: A link to the new resources created and/or fixed in AWS or your own server environment work. For terraform this could be the generated Terraform plan pasted into the MR.
Systems Work:
A example of an Event passing through the system being processed successfully at every stage.
An updated IAM policy that fixes a Service
For AWS I will post Cloudwatch logs, the SNS message or SQS message that was processed, the new Dynamodb entry, and many other things. Almost every service in AWS has some sort of method of tracking and logs and these will be useful to send to your peers.
Deployment Environment work: copies of shell scripts ran, screenshots of the updated /bin folder, updated apache config, etc..
Database work: Example of a new table being created or modified
General screenshots or data samples that allow the reviewer to also confirm that the code works as expected. I go screenshot heavy on my PRs as that is my “Proof of Work”.
Test Cases and Steps to Reproduce
This should be mostly a list of bullet points. This is my own personal favorite section when doing MRs as is it where I double check my work and ensure I covered all my bases for the requirements. If a business requirement isn’t included as a test case somehow that is a MR “code smell” and you will have any developer worth their salt call you out on this.
This section is really critical for me as I often reference older PRs as a way to retest work and confirm prior assumptions of the system
This allows you to get into the mindset of the developer and ensure they have covered every code path
This is the committer’s attempt to also prove they tested their work correctly per the business requirements and also ensure no degradation of the system
Will often focus on manual testing and higher level acceptance testing and E2E testing here as unit tests and integration tests are mostly self documenting in the code review
I try to include raw data here as needed as, often I need a specific JSON structure to test something and having it handy will save me and others time.
Merge Request Checklist
This section will include many of the code review workflow processes to help committer ensure they are following the rules of the repo. Often it will be a checklist you can click off on while creating your MR, but keep in mind MRs are living documents and not everything can be accomplished immediately. It takes time to run tests, have the documentation team write their updates, have the significant stakeholder sign off such as security team members or product managers approve.
Some things will be simple such as:
Formatted the code per repo styles
Updated library version (if required)
Pipeline passes successfully
Follow up E2E story created
Dependencies are up to date
Added appropriate unit tests
Outside of this there will also be specifics on how your organization and specific repo operates. This is where other teams or stakeholders who need to sign off will include their particular needs. This also depends on the type of development work being done. An application developer workload will be different from what a security or networking engineer works on, so adjust the process accordingly. I do fullstack dev so I have to think of the many issues up and down the stack and ensure that the MR process covers those cases. Monorepos will often have significant amount of checks and templates to prevent regression and ensure quality, where as a more targeted repo such as a frontend repo will have less.
Merge Request Assignees
Who you assign a MR to is important. If you have a unified team that works together on most things then you can just assign it to everyone or have a sensible default. Often most repos will have a default and you can just do that, plus add any additional key stakeholders if needed.
Often there are large teams that do silo’d work that we don’t want to annoy. Be tactical here if you have discretion and don’t add everyone. People’s review styles are all different and I know some engineers will nit me to death over silly things, so I often exclude those people as it just makes the process tedious. You should ensure to have a technical expert to review your code and often this person will be the default. If you’re merging code after two juniors sign off you might get some questions from management if your code breaks. Keep in mind MRs are historical records once merged and this can be used against you if something goes wrong, so be careful LGTM comments.
Code Reviews are not a punishment and if you have good teammates they will point out flaws that should be happily fixed. Keep a professional tone always as everyone can see how you act. MRs should be collaborative, not combative on a healthy and well functioning team. If you experience significant pushback you most likely have not cleared a design with the team or your approach is flawed. Try to fight those battles before you write code and not after to save you time.
Real World Example
To make things more real lets look at a real MR on Gitlab’s repo. Ignore all the labels and extra fluff and the automated processes, let’s just focus on the core document.
You can see they clearly give a description of the issue, link to other MRs for context, and describe why they did things a certain way. There are screenshots for proof of work and also code samples on how to validate the feature works as expected. They assigned only two reviewers who both signed off and appear to be the knowledge experts.
This is pretty simple as far as MR docs go, but the code change matches that, which is a good indication. You should scale your MR doc to the level of complexity and changes that are required in order to ease the minds of the reviewers. A 50+ file change of a complex changeset will require a lot more careful review than a single line change, so make the life easy for your reviewers and give them the clarity they need in the MR. There is also a link to a code quality checklist for the committer to review.
Overall this simple but clean example is a good way to get started if you don’t have a process. I would investigate other public Git repos such as Microsoft if you need more inspiration besides this example.
A Sample MR Template
Below is a sample markdown file you can add to your repo under .gitlab/merge_request_templates folder that you can force upon users. Gitlab explains how this process works here. I highly encourage this as it makes is so your MRs are consistent in formatting and also that you don’t have to recreate this template by hand on every MR.
## Summary
Provide a brief description of the features or fixes included in this merge request. Explain the motivation behind these changes and how they impact the project.
## Testing Instructions
Detail the steps required to test the changes introduced in this merge request. Include any relevant scripts, commands, or links to resources needed to perform the testing.
### Automated Tests
- [ ] Ensure all existing automated tests pass
- [ ] Add new automated tests for all new features and bug fixes
### Manual Testing
- [ ] Describe any manual testing steps if applicable
## TODOs
Here's a checklist of common code review checks to ensure quality and consistency in the codebase:
- [ ] Code follows the coding standards and style guides of the project
- [ ] All dependencies are properly managed and up to date
- [ ] Security implications have been considered
- [ ] Code is properly commented and documentation updated
- [ ] All new and modified functionality has been thoroughly tested
- [ ] Performance implications have been considered
## Assignments
Assign team members to specific tasks or roles within this merge request. Tag each assigned team member.
- **Code Review:** @username1
- **Testing:** @username2
- **Documentation:** @username3
## Additional Notes
Include any other relevant information or context that reviewers might need. This could be related to deployment considerations, potential conflicts with other branches, or areas where you would specifically like feedback.
## Screenshots
If applicable, add screenshots to help explain your changes and showcase the UI modifications.
---
Please review the above checklist and provide your feedback. Thank you!You can also assign default reviewers by adding the text to the template above:
/assign_reviewer @user1 @user2 @user3This is particularly great if you work in a mono repo and have different types of PRs depending on the workload and only want to assign to certain people. You can see more commands to add into your MRs here.
Other Tips
Finally here are some additional guidelines to setup in your repo to make MRs more manageable. Your git host should have equivalent solutions and I suggest you worth with your team to identify where the pain points in the review process are.
Are people sending unformatted code? Make the build fail and block the merge.
Are people’s code out of date? Require their MR to be merged or rebase to the latest master before merge.
Here are some more rapid fire ones:
Require pipeline to pass to Merge
Codify expectations of code into the build pipeline.
Almost every language has a de facto CLI tool to format code that can be configured to the team style
Fail builds fast if the MR isn’t acceptable. Can fail on formatting, build, unit tests, and more.
Require approval minimums from mid level or higher devs
Ensures that juniors have atleast one person watching them
Enforce default templates on MRs
This helps external team members too
Enforce default merge style on MRs
I prefer squash merge for clean history
Enforce Fast Forward Merge with required Squash
This removes merge commits and makes the history easier to understand
Squash ensures that many small commits do not pollute the history
You can do a Rebase focused pull to update code. This can be operated in the Gitlab UI also without the CLI to save time.
Use Draft status to clearly indicate work in progress
I often do this as an optional code review for people
Close MRs that need rework to clear the queue to reviewers
Sometimes our initial approach isn’t the best. I like to close MRs that require significant rework to unclutter the MR timeline. This is more of a personal preference.
If working in a large codebase with many people use Code Owners to ensure no stealth edits of your code.
Any extended back and forth on a code review can be moved to a quick call. This is often less painful then commenting and waiting 30 minutes every time for a response.
Just ensure the resolution is summarized in a followup comment
Don’t be afraid to modify your merge template, it’s version controlled and experimentation should be encouraged! Find the best process for your team.
Require more approvals for Merges that lower Code Coverage
Summary
The key takeaway of this article is that your MR’s need to be clear and concise with a focus on sharing knowledge to your team without bogging them down. The less painful this process is will result in faster iterations for your software project. Work together as a software team to build sensible defaults into the MR process and automate as much as you can so that people don’t have to be broken records fixing easy mistakes. By following the advice in this post you can make your life and every else’s life better when it comes time to merge code. If you’re submitting multiple MRs a week this is a must and you will reward yourself over time by building a good process for the team.