Don’t be Desmond, or How experimenting with Trunk Based Development increased team trust
Desmond was a character in Lost* who was trapped in a bunker, having to enter the same code into a terminal every 108 minutes or “something bad would happen”. He becomes so obsessed with it that the important thing becomes entering the code, not preventing “the bad thing happening”.
* The show everyone thought had the most disappointing ending before Game of Thrones was a thing
Context
When the Wolves team was put together in January, we took the time to put together a charter and set of working practices.
We adopted the pretty standard (for Redgate) way of getting new changes customers:
- We only release from `master`
- `master` should always be releasable
- We would use Pull Requests (PRs) to get code changes into master
- Code would be reviewed by a peer before merging
- Branches would be short-lived (preferably hours, maximum 2 days)
- PRs should be a small
- PRs should be speedily merged (again, preferably hours, maximum 2 days)
- We prefer pairing over solo work (almost everything should be paired on)
- Use common sense
We automated as much as we could, checking in team shared Resharper settings and using Prettier to format all JS, CSS and, HTML on save.
David H put together a little Slack Bot response to randomly generate a reviewer whenever we typed `pr4cr` (Pull Request for Code Review) into our team channel. Slack Bot wasn’t the most random thing in the world, and fairly often suggested the same person requesting the review, but it was a super lightweight solution to help prompt us to make sure code was reviewed quickly.
Everything was good, all code was getting reviewed and we were fulfilling our working practice agreements.
Ask the experts
Back in February, ThoughtWorks came in to help us scope out a new project, whilst they were here, we took the opportunity to quiz them on Trunk Based Development. I had a reasonable understanding of what TBD was (just push straight to master :trollface:), and it felt like (other than using PRs) that was how Wolves were operating. I went along to see if there was something I was missing.
Well, it turned out, we were… ThoughtWorks didn’t use PRs, but so long as the outcomes were the same (We had (some) tests and were using CI, used short-lived branches, always merged to master, etc) it didn’t really matter.
Everything was good, the team was essentially doing TBD already. Smashed it.
The end?
The SQL Census team were a pretty small: for most of the time we were only 3 engineers + a tech lead. A lot of the time we operated as 2 pairs, making sure everyone got a chance to pair with everyone else.
We were iterating really quickly and code was churning at a crazy rate. Part way through some work, one of a pair would get assigned a PR from the other 2, they’d excuse themselves to look at the PR (or be poked the PR’s creator), add some comments and go back to their original pair.
I’d disappear off to a Tech Lead meeting or 1 to 1 and be assigned a PR that’d sit there until I got back.
Little PRs like moving a button, rearranging some UI, cleaning up a few tests would get assigned and sit waiting for the assignee to give it a cursory glance and merge. They were quick to merge, but hey, it’s only a minute, and it makes sure we get a pair of eyes on every change.
Everything was fine: We were following our working practices, we were essentially doing TBD, but things didn’t feel quite right.
The Early Access Program for SQL Census kicked off with a bang, we were having 3 calls a day and we were trying to rewrite the whole UI in between. The team was feeling stretched.
Context switching between a piece of work, reviewing a PR and adding comments, going back to the original task, get replies to comments and eventually merging was a real distraction. Factor in having customer calls and all the other regular 121s and things and it was becoming exhausting.
We had forgotten about our last working practice: Use common sense
To PR or Not to PR
Could we reduce context switching and at the same time reduce total consecutive PRs whilst also reducing how long they were open? We were already pairing by default on every task… there were 2 pairs of eyes on almost everything, and who gives a damn about seeing UI tweaks. Why would you submit a PR if you hadn’t tested it? Even if it wasn’t right, it’s easy to roll back or forward.
We decided to try out some TBD, we decided not to over think it and just give it a go: We’d “fake it until we made it”. Yep, it’s cool to push straight to master, we’ll give it a go and see how we get on for a couple of weeks.
Yeah, well you see how that went… fail fast, fail often…
I was breaking master because I was used to relying on CI to check my changes and running tests locally means sitting about waiting for the tests to run before I can move on (even if it is only a few 10s of seconds). I also missed how I could quickly look back at what happened whilst I was away by looking at closed PRs and how, when I’ve finally made my millions and retired, the next person to look at this code would at least have some context as to why they change was made.
We took a step back to set some ground rules:
- Always start work on a branch (yes Parker, I’m looking at you)
- If you want a review, create a PR and request one
- If it’s a single commit, merge it straight to master and push
- If you’ve got a group of related commits:
- — Create a branch with a “What? Why? Tested?” PR
- — Merge when green (yep, merge your own PR)
- We’d introduce a fortnightly 1hr lean coffee team code review session to look at how we could improve/standardise
- Use common sense
Everything was good: We were following our working practices, we kept our contextual history of changes, kept master green and reduced context switching, consecutive number of open PRs and PR review length.
Something else weird was happening too — The team suddenly felt more solid, there seemed to be more trust to just let our awesome team members JFDI…
Don’t be Desmond aka Trust Your Team
So it turns out that we were essentially already doing Trunk Based Development — and you probably are too! Kudos! The only thing slowing us down was the idea that a Code Review had to mean a PR. Once we got past that idea, days felt more productive and less fragmented.
Code review and PRs have become such a given that they’re almost invisible to us. It feels like they’ve become the most important thing, rather than just being a mechanism to help stop “something bad thing happening”.
Don’t be Desmond. Maybe it’s time to review how your team uses PRs, and see how you can make your days feel more productive and less fragmented too!
