Tag Archives: Git

Git Basics: Pull Requests

By Agustin Aliaga – Developer at Santex

Git Pull Requests workflowGit is a powerful version control system designed to make software development collaboration easy. It can be used for personal (single contributor) repositories, but it really stands out in projects where multiple developers modify the same codebase every day. Multiple branching models can be adopted. However, in most cases, a Pull Request (PR) will be created when a bug fix or a new feature is ready to be merged into the main branch. The PR itself is an attempt to merge specific changes from one branch to another. Let’s explore the Pull Request workflow in detail.

PR Creation

Whenever a PR is created, the following best practices should be taken into consideration:

  • Add a helpful description to the PR that answers the following questions: What is the requirement or bug? Is there a link to the issue (e.g. on JIRA)? How does your code fix it? Is there anything else the reviewer should take into consideration?
  • Make small, consistent, and logical PRs: We want our PR to be merged as soon as possible. Imagine how hard it would be to review hundreds of file changes at the same time. If this happens, it is likely the reviewer won’t have, or take, the time to do it properly. Do your best to keep it simple and concise.
  • Make sure the PR’s metadata has been set. For example, this can be done by assigning the reviewer (to trigger a notification), setting the assignees, or adding a label (if needed), etc.
  • Configuring the repo’s branching security settings to keep developers away from self-merging their PRs or pushing to a shared base branch. To avoid this, I recommend enforcing Github’s branching security settings. When a large team of developers are collaborating on the same repo, there is a good chance of accidents happening. A single mis-click in a “merge” button can cause terrible headaches. Protecting important branches is something I’d advise most of the time.
  • Avoid submissions that include unresolved conflicts. Fixing those conflicts shouldn’t be a reviewer’s responsibility. PR creators should dedicate time to solving them, either by using Github’s conflict resolution tool (when it’s simple enough) or by using your favorite diff tool locally. You can achieve this by pulling the base branch and merging it into your feature branch. After you’ve pushed it to the remote repo, the PR will automatically update.

Automated tests

If a Continuous Integration system (such as Jenkins, Travis, or CircleCI) is set up, a bunch of hooks can be configured to run unit tests on PR creation. This will allow the team to detect and catch bugs rapidly, as well as prevent conflictive code from being merged. This is a long topic that requires its own blog post, so I’ll just move on to the following stages.

Code Review

After everything related to the creation of the PR is dealt with and the CI tests have been passed, it is time for a code review. Projects tend to have tight deadlines, which can sometimes make code reviews seem like a waste of time to other team members or external agents. Ironically, code revisions can actually help increase productivity and reduce reworks because we avoid bad practices in our code and share knowledge between contributors.

Some benefits of implementing code reviews are:

  • Less experienced developers get to learn from their mistakes.
  • Experienced developers can consolidate their knowledge as they teach others.
  • A high-quality codebase is ensured.

Humility: a soft skill that matters

Sometimes, as work starts to accumulate and things get tense,  engineers have a tendency to become less aware of their attitude toward their peers. It is always important to avoid egocentric behavior, listen to our co-workers, and moderate our communication when reviewing other peoples’ work. If we write a PR review in a disrespectful/arrogant manner, we could be damaging the teams’ confidence and the work environment.

The “reviewer” role

Ideally, teams should implement peer reviews. This means that anyone who has the required experience and skills should review others’ code. In practice, collaborators regularly have different levels of experience in both the technology used for the project and the codebase itself, including its set-up, architecture, code styling, deployment, etc. In this case, experienced developers should be conducting the reviews and newer team members should be included as they progressively get more comfortable with the project.

Merging the PR

After the PR is approved, it’s time to merge it. We have a couple of options for doing so. Let’s explore them:

  • Create a Merge Commit (default): This option will merge all the commits from the feature branch, plus a new merge commit. This is the safest way to perform the merge, since it is a “non-destructive” operation. The downside of using this option is that, since it creates a merge commit by tying together the history of both branches, it pollutes your history tree with multiple “irrelevant commits.”
  • Squash and Merge: This option will “squash” all the commits into a single commit. If the PR includes a lot of commits, this would be the most practical way to approach a squash and merge. It will make your history tree much cleaner and easier to read in your base branch. However, you will lose granularity due to the bigger size of the resulting commit.
  • Rebase and Merge: The “rebase” operation is another way to combine commits from two different branches. This will put your feature branch commits on top of your base branch’s latest commit, effectively rewriting the commit history. After this, it will perform a fast-forward merge, making it look like all the work was done on the base branch. This is extremely dangerous when the rewritten branch is public and shared with other team members. Generally, the rule of thumb is to keep rebasing operations for private-only branches.

References

 

Good committing

By Jose Torres – iOS Developer at Santex

Good git commitPeople in software development know there are a ton of difficulties that a team can face when working on software projects. That’s why best practices were introduced: as a way of preventing  common problems that software teams will eventually face. Best practices cover many aspects, from the setup of the environment to the way we write code and produce deliverables. Unfortunately, one aspect that is usually forgotten or underestimated is the handling of the repository. This usually happens when the team is small – if you are the only member on the team, you are bound to be disorganized. This can hide other potential issues that are usually faced when the team grows and you find yourself in complicated situations or flows. As the team increases its size, a lack of best practices will reveal itself and problems will surface.

As engineers, we should understand the importance of doing things right from the beginning. It shouldn’t matter that our current context shields us from facing complex situations. We have to be prepared for when the context changes and things start to scale. It often happens that engineers have this feeling of “things are working fine,” so therefore the context is “that should mean we are doing things well, right?” Not facing issues in the moment is not a guarantee that things are being done right.

There are many areas of best practices surrounding Software Configuration Management. Let’s focus on some of the best practices that should be used when committing code.

When committing code, we are introducing new code changes to our code base. In distributed version control systems, this is done first in our local repository and then the code is applied (“pushed”) to the central repository. There are multiple approaches to handling the branching model of a repository, but we will focus only on committing. It is basically a general rule that a commit in code should reflect an atomic change. This means that it can only be applied entirely, never partially. And, of course, it should not break the build. All build routines and tests should run successfully.

When committing code, we also have to include a commit message. This message is meant to give more information about the code changes that have been introduced. Such a message will last through eternity, and it is our duty to use it wisely.

Good git commit

Image 1: Sample git log

Limit your commit first line (title) to 50 characters

This is one of the most common suggestions, but it is still often forgotten. The first line is treated as the title of the commit. This means it should be brief and concise. It must be no longer than 50 characters. On the flip side, it should not be too short. The character length may not be a hard limit, but having it in mind ensures that the author thinks for a moment on how to concisely describe the commit. This helps to get quick contextual information when navigating through the git history and ensures that all clients will display the commit info properly.

Consider a blank line before the description

In order for a Git to differentiate the title from the body of a commit, a blank line must be added. Of course, this is only required if we want to add a description. Whether a description is needed or not is up to the author. However, it is a good practice to include a description that adds an explanation of the changes introduced and also some contextual information, like a related ticket number or link.

Include a link related to the issue on Bitbucket/GitHub/JIRA/etc.

This is related to the previous suggestion. It is important to have a reference to the source of the request, so we can have more context of the change overall. This should not be part of the title of the commit, but rather part of the description with the full link. If it is a bug fix, requirement, enhancement, etc., most of these are tracked with software development tools that assign them a unique identifier which makes it easier to track development. On one hand, it helps each commit in the repository to have a full reference. On the other hand, it gives visibility to external tools about when, by whom and in which branches the changes are applied.

Write the message using present tense/imperative mode

Finally, the golden rule of committing: all the writing must be done using an imperative mode like [FIX] and not using past tense like [FIXED].

Define a template

Having all of the previous considerations in mind, the team should define a template. After discussing what should be included in the ticket as the description and syntax of the titles, your team will have a clear idea of how they should commit code. Here is a sample I used for my current project:

Git commit - Sample git log

Image 2: Sample of a git commit structure

All of these best practices will be useful in many of the situations that software teams often face.

Help during the review process

When doing code reviews, we have to check all of the new code that has been introduced. Having a clean commit history with descriptive titles and useful messages helps us to understand the reason behind the changes. Also, as the reviewer will have access to the description with a link to an external tool, he/she is able to understand the full context of the change.

Help when handling merge/rebase operations

This is my favorite. Every time we have to deal with rebase operations it is extremely useful to have a clean commit history. This helps to make sure you are moving the right code changes from one branch to another. Without this information, it would be almost impossible to do without having assistance from the original author, which we know may be impossible sometimes.

Help when writing release notes

When preparing the release notes, a clean commit history is useful so we can see all changes that are part of our release branch (or any branch we are using to prepare a release).

Help during maintenance

Finally, one of the most important reasons for having a clean commit history is so that we can understand the changes that were introduced a long time ago. Often as software engineers, we are often in a situation of not understanding why a specific change was made or why a piece of code was originally introduced. A well written commit will bring together all of this information, and will give important insight to the new authors, allowing them to avoid having side effects when performing a change.

Sample of a git commit structure

Image 3: Sample of a rebase operation for moving code between branches

There are a ton of recommendations and conventions for good committing. These were just some of them! These are the ones I think are the most basic and need-to-know, and are based on the official Git Man page1.

Let me know if you have any others you would like to add or suggest.

Links:

– [1] https://mirrors.edge.kernel.org/pub/software/scm/git/docs/git-commit.html#_discussion