A Look Inside: The Path to Trunk
Getting changes to our mainline source repository (aka trunk) has never been easy, but the problems scale with the amount of people trying to merge their work. We have grown quite dramatically over the last couple of years, and the work on new features, improvements, bug fixes, refactorings and, of course, automated tests is always in full swing. At any given time, we have hundreds of open Pull Requests against trunk.
As you might already know from our previous blog post on our QA process, we tend to work in separate branches, where all changes are subjected to an early in-branch testing. So what happens next? How do those changes actually get merged to trunk or other release branches?
The first thing to do is open a new Pull Request (PR) in Ono (our source code management system, based on Kallithea), add a detailed description that contains the purpose, testing status, technical risk and any other relevant comments. Every PR needs to be reviewed by a peer who has domain-specific knowledge for each area it touches. They give feedback on the changes. Ono has a system that automatically suggests who should be invited to review certain parts of the code.
The code reviews are a critical part of our development process. The reviewers focus mainly on following questions:
- Does the code achieve the intended purpose?
- Does the code have good test coverage?
- Does the code needs manual testing and has it been performed?
- Is the code well-written and does it follow the Boy Scout Rule (“Always leave the campground cleaner than you found it.”)?
- Does the code follow the coding standard?
The number of people involved in reviewing the PR may vary, depending on the amount of changes and areas being touched. It can get to as many as 10-15 people for bigger features. Based on this feedback, we edit the code until everybody agrees on the right approach. So we often add only team peers at first to gather the initial feedback and fix all found issues before involving a wider range of people. Also, we like to split PRs into smaller chunks with isolated changes, since those require fewer people to review them and the reviewers can look at the code more thoroughly.
When the final PR is ready and went through the last round of reviews, we run another round of tests. Usually, we run the set of build targets and automated tests suites that are most commonly/easily broken. We have special targets set up on our build automation server Katana to make this a one-click process for developers.
For the final PRs, we also have to obtain an approval from one of the Release Managers.They assess the release risk and make sure all the right parties have discussed and seen the changes.
Once all the green dots have been collected and the tests have returned green, we can take the PR to the next step and add it to the Trunk Merge Queue. Voila! The PR is now waiting to be merged.
It might seem that this is it, but it’s not the time to relax yet. Before being actually merged to trunk, the changes need to pass Trunk Queue Verification! We introduced this process to improve the stability of trunk. So what does that mean exactly?
Each batch of PRs is automatically merged on top of trunk in a test branch. Then it’s pushed to a draft repository. Then more builds are launched and we are run all tests. An automated system polls our build system, analyses build results and sends status messages to an internal Slack channel.
If a build or test fails, it will first be launched again, both on the same machine and on another one to see if it is a stable failure or an instability. A failure will also initiate a bisection: a build will be launched half the way to the last known working revision. Repeatedly starting builds in the middle of the untested range, the system finds the PR that introduced the failure and pings the owner in a Slack channel.
The offending PR is taken out of the batch. Then, the good part of the batch is automatically merged on top of the trunk and pushed. Owners are notified about this happy event by a ping in a Slack channel and the PR is closed.
Why do we go through this whole process? We aim to improve the quality and stability of Unity! Following these steps helps prevent bugs and test failures from reaching trunk.
How do you test code in your team? Let us know in the comments!
If you’re passionate about code quality and would love to contribute to making, testing or maintaining Unity, join us!
31 CommentsSubscribe to comments
Comments are closed.