11
minutes
Mis à jour le
26/12/2022


Share this post

Learn how to effectively review and merge code changes with these tips and best practices for mastering merge requests and pull requests.

#
GitLab
#
Github
#
MergeRequest
#
Performance
Julien Travers
Software Engineer

Introduction - What is a Merge / Pull Request

As a software developer, you will have to perform pull and merge requests in your day-to-day work. So, it’s important to know how to do it well.

Moreover, this phase of the workflow is one of the first barriers between bugs and the production environment. It also keeps a record of what has been done which will be very useful to understand the context of the code’s creation. However, if it’s not done properly, it can become a very painful step for your reviewer(s) and increase significantly the lead time of the code review. In this guide, I will show you some good practices that will bring your merge requests to the next level!

👟 Basics

Merge or pull requests are created in a git management application and ask an assigned person to merge two branches (ex: feat-branch with dev, dev with preprod or preprod with prod). Tools such as GitHub and Bitbucket choose the name pull request since the first manual action would be to pull the feature branch. Tools such as GitLab and Gitorious choose the name merge request since that is the final action that is requested of the assignee. In this article we'll refer to them as merge requests.

💡 A "merge request" should not be confused with the git merge  command. Neither should a "pull request" be confused with the git pull command. Both git commands are used behind the scenes in both pull requests and merge requests, but a merge/pull request refers to a much broader topic than just these two commands.

🔀 Merge Request in a “classic” Git flow

There are multiple steps that you have to complete before pushing your code into a production environment. To be clearer, I have drawn for you a standard Git flow to show you where a Merge Request is placed.

Untitled-2

🙋‍♂️ Owners and actors

During the code review on your merge request, you are going to have interactions between few people:

  • Your tech manager (in my case it’s a tech lead, but it could also be a Lead Dev, an engineering manager…) ⇒ First, they will give you feedback about your way of coding and suggest others approach to improve your skills. Second, they will anticipate potential bugs that you might have created and check that your code is maintainable.
  • Code Owners (if you change some lines that are not in your teams scope) ⇒ they will anticipate potentials bugs and breaking changes that you could integrate without being aware.
  • Your teammates (and you) can also review your code. It’s a very good exercise ⇒ It can improve your skills on different languages and also step up your level on reading and understanding algorithms quickly… Which is very convenient when you have to work on new code bases.

 

Good practices

Precious time from my tech-lead was lost because of my dirty merge request. Top of that, I also lost many hours trying to change lines of code and commits after reviews from my manager. However, thanks to the experience that I have from working on different projects for big clients, I have acquired good practices that I will share to you.

✅ Use templates

Merge request templates are pre-defined layouts that you can use when creating a merge request within GitLab (or Github). If you have them set up in your project repository, then they are available during the merge request process.

🧾 Information that you must have in your template

  • A clear description section:

    You are going to send the merge request to reviewers. To decrease the lead-time for the code review, the reviewers need to have the context and all the information needed in 1 click. In that way, the description needs to be exhaustive containing a Why? and What?Part as well as a link to the user story (jira or trello). You can also add screenshots of architecture schema and other graphs to be more explicit.

    Finally, you can add a brief summary of what has been done (if it’s not obvious from MR’s title)

  • A template example:

    Big warning for this part: If your template is too restrictive or too long to complete, the developers will leave boxes blank. So, keep it small!

    Here is a template that summarise the previous section

    👌 Tech tips

    • Make it small:

      If you read this article from Google about merge and pull requests, it talks about the maximum number of lines of code that you must not exceed. The number is not important because it depends on the complexity of your algorithm or the programming language. But you have to keep in mind the approach:

      • The more your reviewers are juniors, the more your MR needs to be small. After reading few articles on internet, they all agreed to say that common answers range from 60 minutes to 2 hours. Exceeding this amount of time is too much. On top of that, according to this article from smartbear and this one from quora, a MR must not contain more than 500 lines of code.
      • Split big features into small ones.


    • Conventional commit:

      • Conventional commits are a standardized way of writing commit messages that follow a specific format. It typically includes a type, a scope, and a subject. The type specifies the kind of change, the scope specifies the affected code, and the subject is a summary of the changes. There are various conventions for writing commit messages, such as the Angular Commit Message Conventions and the Conventional Commits specification. These conventions provide guidelines for writing clear, concise, and consistent commit messages.

        💡 If you want to check that commit messages follow “conventional commit” rules, you can set it up using this tutorial.

    • Communicate:

      • If there are any owners of code sections that you want to modify, discuss with them about your approach before developing the feature.

    • Start from a fresh version of the code:

      • Create your branch from an updated branch (master, main, staging, prod, preprod, dev) by doing a git pull command
      • Don’t forget to rebase using git pull --rebase origin <branch> regularly to avoid potential conflicts. It will save you precious time and bugs.

     

Standard errors

 

⛔️ Thinking that Git Hooks are useless

Git hooks are shell scripts that can be found in the hidden .git/hooks directory of a Git repository. These scripts trigger actions in response to specific events (like git commit), so they can help you automate your development lifecycle.

They are very useful when you want to integrate rules that every developer must respect to be able to push code.

💡 A good way to start is by defining a pre-commit hook with husky that will perform:

→ A coding style check and apply a linter

→ A commit message check (ex: git commit -m "<type>(<scope>): <message>")

🔨 Quick tutorial to set it up

Step 1 : Init a npm project with a package.json file and add husky.

Step 2 : Configure your husky inside your package.json file previously created

Now you can apply customs bash commands for each hooks.

❌ Pushing one commit once a day

A lot of programmers are doing a single commit per day and/or per Merge Request. They are a few reasons for this :

  • They don’t know exactly how they will develop the current feature (so they are waiting to finish before commiting)
  • They don’t know how to easily change their previous commit (messages and content)
  • Be afraid to be not atomic (I will explain this concept below)
  • They don’t know if it’s a bad practice or didn’t think about it

A good way to commit a good merge request and rewrite the history of what you have done is by doing an interactive rebase.

Hard Spots

 

⚛ Atomic Commits

When developers discuss clean code, they often mention the single responsibility principle. It’s possible to extend this principle to Git. Indeed, we can define an atomic commit as one that can be picked or reverted without any unwanted side effects or regressions. If a commit is removed from your git commit history but doing so removes other legitimate changes, then that commit wasn’t atomic.

💡 A good way to know if your commit is atomic, is just doing a git cherry-pick command without creating errors in your code (and without breaking your test suite).

You can make atomic commit by doing 3 things:
  • Creating a schema that shows how you will develop your feature (exemple for a new backend endpoint)
    • Step 1 : formalize in json the data structure that you will manipulate in INPUT and OUTPUT (don’t forget to formalize also the INPUT and OUTPUT data structure used by others apis that you could call)
    • Step 2 : Verify the scope of your route → Do I need to create a new controller? If yes, where? Which rules do I have to follow? Domain Driven Development?
    • Step 3 : What about the Service associated to the controller?
    • Step 4 : Implement the nominal case
    • Step 5 : Which edge cases do I have to handle?
    • Step 6 : What are the error codes that I have to return?
    • Step 7 : Do I have a plan to test my code during the development?
    • ⇒ Commit 1 → step 3-4
    • ⇒ Commit 2 → step 5-6
  • Commiting files with their corresponding tests
  • Using interactive rebase to rewrite the git history and rearrange what you have done if needed

✉️ Commit message

I talked about conventional commit earlier… However, it can be very complicated to be explicit in your commit message because you have to synthesize a complex development in just few words.

A good start to be better at writing commit message is by putting yourself in the place of a journalist :

  • Use the present tense
  • Be factual
  • Keep it short: Commit messages should be concise and to the point. They should not exceed 72 characters per line.

💡 The complexity to explain what you have done in your commit can be a clue to show that you have not split it correctly. You may have done to much in just on commit.

🎒 Starter pack tools

A lot of tools exist to help you manage your commits. These are just a few ones that I use in my day to day work as a software engineer :

🪛 LazyGit

It is a terminal based UI tool for Git commands. This tool allows you to view commit history, pull or push changes, resolve merge conflicts, checkout recent branches, and more.

🔧 Integrated git tool in JETBRAINS IDE :

By default, you can access to it via the Git tab at the bottom of the IDE. This tool is very useful and convenient to handle interactive rebase.

🔨 Integrated git tool in Visual Studio Code

You can use integrated git tool in vscode to manage your file (add, delete, revert), commit your work or check git history. Top of that, a plugin called git lens is very useful to watch the project’s git graph (cf picture 2 below).

Git lens / Git Graph

 

🏁 Conclusion

In conclusion, mastering merge requests is an important skill for any software developer. By following best practices and using the right tools and techniques, you can streamline the code review process and ensure that your code is of high quality and ready to be merged into the main codebase. Some key things to keep in mind when working with merge requests include:

  • Communicate with owners
  • Use Pre-commit hooks tools → to apply linter, prettier rules and check commit messages
  • Take the time to fill your Merge Request template
  • Use tools and test automation to ensure quality
  • Collaborating with your team and being open to feedback

By following these guidelines, you can become an expert at working with merge requests and contribute to the success of your development team.

You will decrease the lead time for the code review and increase the code’s quality introduced in production.

 

📖 Sources

How to Make a Perfect Pull Request - Senior Software Engineer at Workato

How to write the perfect Pull Request - Technical Advisor to the CPO at Stripe

The anatomy of a perfect Pull Request - Engineering Manager @ Sketch

Writting a good Pull Request - Guide by Google

How to Write Better Git Commit Messages – A Step-By-Step Guide - UX Engineer at D2iQ

Les erreurs les plus communes - Unknown

Merge Request template - check list for everyone - Senior Developer at ECigaretteDirect