Git and Github Usage
Using Git and GitHub in your Android team
At Freeletics we use Git and GitHub on a daily basis as vital tools for all our crucial projects. To help the entire engineering team contribute fast and effectively, we abide by a set of rules that we partly developed by ourselves, and partly imported from best practices in software development. Here are a few that you might find useful in co-ordinating your own workflow among your engineering team.
- master - reflects the current app store version
- develop - reflects the latest stable development changes
A branch name should consist of the ticket number (if available) and a meaningful name. A dash should be used as a separator between words. Branch names should be lower case only.
Examples of good naming:
Some of the branches can contain prefixes like:
- release - release branches (e.g. release/3.4.1)
- hotfix - when it’s a fix on production (e.g. hotfix/4.0.1)
Those prefixes will trigger a release job on our CI server Jenkins, with each push to the branch.
If you are the creator of a branch, you are also the owner. You are responsible for taking care of it over its entire lifetime. Responsibilities include:
- Rebasing on a regular basis (make it a habit every morning so you don’t have any merge issues later on).
- Make sure that any Localization changes are kept up-to-date, especially in release branches.
- Deleting the branch when it’s no longer needed.
- Only force-push a branch if you are the owner.
- All other cases need the involvement of all developers contributing to that branch #communicate.
- The branches develop and the master should never be force-pushed. For additional safety they are also set as protected branches on GitHub.
The whole system of Git branching is based on the widely used model of Vincent Driessen.
- Use a short meaningful title (max. 80 characters)
- Use the description for detailed explanations
- Use International English
- Use present tense in imperative form (example: Add translations for Invite Friends screen).
- Use sentence case
- The ticket ID should be mentioned at the end of the description (separate line), not the title. Such meta data will work with the integration of the Bug Tracker system.
- Squash commits when it makes sense
- Commits should be atomic
Example of good commit message:
Add translations for Invite Friends screen JIRA-713
Pull Requests and code reviews should ideally be just a quick sanity check of your work. Remember this at all times and the process will be as hassle-free as it can get, for all developers that are involved. We use GitHub for reviewing Pull Requests, so some of the items belong to GitHub features and could be missing in other code review tools.
Why are we doing Pull Requests?
- Eliminating bugs at an early stage
- Improving code quality
- Increasing knowledge about the project
- Learning from each other
- Making sure that our definition of Done is met
- Not about naming and shaming < this is crucial
- Shorter Pull Requests = faster integration
- Avoid bulk commits by issuing small, atomic commits
- Quality means sticking to the same process for every single Pull Request (use templates).
- Get your architecture and design of the feature and fix challenges by the team before you even start coding. This avoids long dicussions later on, conversations that may involve further Pull Requests.
- Choose the correct target branch
- Remember to use the Boy Scout Rule whenever you can.
- Assign and request a reviewer. Assignment shows in the overview of the Pull Request. An unassigned Pull Request means that it’s free to pick up.
- Before assigning, review the Pull Request yourself (check all points on the template).
- Communicate assignment to your team mate. Don’t rely on GitHub notifications #communicate in a separate channel (e.g. Slack)
Think of this before doing a Pull Request on develop: The feature or fix should be production-ready, so can be part of the next release.
In order to go through the list of Pull Request (PR), we have a different group of labels:
- beta – the PR should be merged to the current beta (next release) branch
- hotfix – the PR should be merged as a hotfix to hotfix branch
- wip (work in progress) - the PR is not ready to be reviewed in detail but the creator already wants feedback
- ready – the PR is ready for review and can be picked up
- blocked – the PR is blocked and cannot be reviewed at the moment
- Complexity. Defines the complexity of the PR in order to evaluate the PR faster
- high - If complexity is high, it alarms that probably something is wrong in the PR and creator should revise it.
- Team. Defines to which feature team the PR belongs. It helps to split visually PRs by different logical groups.
Keep the wip Pull Requests to a minimum, especially if they don’t at this stage need to be visible. This will give a better overview and saves the build power on Jenkins for PRs that actually need it. Remember that ideally you get your concept, architecture or class design challenged before you start implementing
- As a reviewer, communicate when you start and are done reviewing
- As a creator, don’t push while someone else is reviewing #efficiency
- Stay friendly and constructive during code reviews (a good source on the right tone)
- If dicussions get too long, take it offline (and add the results in the PR)
- If you find an inspiring piece of code by a colleague, share it with the team
- When you are done with review, submit it (either as approved or as requiring changes)
- Unassign yourself and assign PR back to creator
- Approval is done by 👍, or 🚢 or :shipit: or by submission review as accepted (using GitHub review functionality).
- Merging should only be done by the creator of the Pull Request.
- Do not merge without review by other team mates (or while any comments remain open).
- All items in the Pull Request template should be checked off.
- Must be tested on a device
- Code must not be merged before CI builds successful
- Do not merge if unit or integration tests are failing
- Use a clear commit message
- We decided to use GitHub’s squash and commit functionality to have a linear history and to avoid having all commits from the branch. But feel free to use another option.
- Delete the merged Pull Request branch.
Pull Request Templates
Pull Requests are inspected by different team members, allowing for an increasing amount of test criteria to be covered. The lists below create an important guide for developers to systematically test. For your daily work, stick to the Pull Request templates. Using a
Strikethrough, cross out any items that might not be relevant for the specific Pull Request.
Example of the PR Template:
- Added tracking events for Wear Popup
- Go to Debug Panel
- Choose item Popups
- Select Open Wear Popup
- Wear Popup should appear
- Check logs for tracking events
Links to relevant tickets
Definition of Done
- Ticket number in commit message
- No warnings added (Sonarqube, Lint)
- Tests added
- All (unit) tests pass
- Test coverage should not drop
- Tested on device
- No regressions (no crashes or critical bugs)
Reviewed by designer/PM
- Changelog updated
Strings added to mobile-localizations repo in <_insert here="" the="" link="" to="" PR="" in="" mobile-localizations="" repo_="">
All that’s mentioned above will always be supported by as much automation as possible. For instance, like using linters to reduce the amount of attention needed to focus on style violations all the time. We always challenge ourselves to see what else can be automated.