Our Git Workflow
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
- 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
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
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 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.
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
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.
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 PRs?
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 to 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. 🙂
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 (
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.
We are planning to automate checking for correct class names with a bash script. Coming soon™
A whole book could be written about this topic. Max wrote a blog post about improving readability of JS code, which might give you some guidelines.
In general try to find changes, which might require updating the documentation. Ask questions about confusing or complex code. This can be done as GitHub comment 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 then we 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.