Our Git Workflow

We use git for version control on all projects and host the repositories on GitHub.

🔀 Our branching strategy

We keep it simple and use GitHub’s flow:

Create a new branch from master
Add your changes on this branch
Open a pull request and request reviews
Discuss your changes
Merge into master, once it got approved

On most projects creating or pushing to a branch will create preview deployments so that changes can be reviewed just by opening a link.

We also have a production and staging branch which will automatically trigger a deployment once new code gets merged into them.

🏷 How should I name branches?

We currently don’t have a specific naming scheme, since we delete all branches after merging anyway and still have their changes documented as pull requests on GitHub.

So there is no need to prefix them with a feature or fix prefix for example. Nevertheless you can do so if you want. It can be handy for Tower users, because prefixes create folders its UI.

⏱ When should I create a pull request?

This is up to you. 🙂

The earliest moment would be when you’ve created a new branch, which has new commits on it. The latest would be when you want feedback to the code you’ve written.

❓When should I request a review?

This is hard to say, since it requires the definition of "done". Nonetheless here are some things that might help with finding the right time:

after you’ve proofread your PR
after you’ve tested it on the feature branch/preview deployment yourself
when you think you’ve implemented all requirements

🔢 How is the review process on GitHub?

reviewer leaves a comment
author fixes the code
author re-requests review or comments on fix
reviewer resolves comment or approves PR

🔀 How do I deal with branches that are not based on master?

In general this should be avoided if possible. From a product perspective there are often ways features can be implemented independently from each other.

When creating new branches, make sure to base them from master. If your branch depends on changes of a different branch, try to get it merged first by reviewing or working on its pull request. Even better would be to come up with a solution, which avoids depending on it. The concept of feature flags can help sometimes.

When there is no other way than to depend on a different branch than master, there are a couple of rules, which will make it easier for everybody.

Example

master ← first_branch ← second_branch

👉 When creating the pull request for second_branch, set its base branch to first_branch, so that only the new changes are shown in the diff. Also add a bold sentence to the top of the PR description, that highlights this branch does not depend on master.

If your branch gets approved, but first_branch hasn’t yet, wait for it to be merged first. Make sure to set the base branch to master again before clicking the merge button. GitHub usually does this automatically.

Exception: In case your changes are very small you can also merge your branch into first_branch. All your changes will be added to its PR though. This can lead to people reviewing your code twice, so usually we avoid this.

⏰ When should I review pull requests?

Currently we try to do reviews during fixed times of the day, so that nobody needs to be interrupted by a review request.

Review times are the following:

first thing in the morning
right after lunch

So when you create a pull request, you can expect that somebody will review it after lunch or tomorrow morning. 🌅

📚 What is a good size for pull requests?

This is hard to answer, since it can vary a lot depending on "how big" a feature is. In general the smaller the better, but splitting up logic that belongs together should be avoided.

Since most of our projects are already organised in components a good indicator is also to have one component per pull request.

🕵️‍♀️ What should I look for when reviewing pull requests?

Before giving feedback, please have a look at our pull request guidelines, so that your review is received in the best possible way. 🙂

CSS

The best way to review CSS is opening the preview deployment and testing its functionality regarding adaptability for different viewports and content.

When it comes to the code itself, check if it adheres to our CSS architecture.

Here is an excerpt:

check if class prefix is the component name
no tags are used as selectors
styles of other components are not overwritten

There is a best practice to move layout related properties to the top of the declaration (position, display, margin…). It’s great when you do this when writing CSS. When reviewing we usually don’t request changing the order, since its not that big of a deal.

Erkal built a linter which automatically checks the correct usage of class names in CSS files.

JavaScript

A whole book could be written about this topic.

In general try to find changes, which might require updating the documentation. Ask questions about confusing or complex code. This can be done as a comment on GitHub or in person. Try to think of error cases and check if they are handled gracefully.

🔃 How should I merge branches into master?

GitHub offers three options: merge commits, squash merging and rebase merging.

We decided to use merge commits, since they don’t delete information like individual commits/messages. A clean view of the master branch can still be achieved by filtering the commit history by commits starting with "Merge branch".

If you have the required access rights feel free to disable the other two options in the repository’s settings.