Code reviews for scope control

We got bit recently. A team member made changes that were outside of the current sprint scope and had been marked as on-hold. Those changes ended up breaking part of the application at the exact wrong time.

In spite of all the moving pieces that weren't written down that caused this to go bad, one thing really could have prevented it: a code review.

As a team, with Bitbucket as our source control platform, we decided that pull requests between branches would serve as a code review opportunity. It made sense to leverage the tools as capably as we could and the approval workflow allows us to create checkpoints for all work. During the changes mentioned above, I used this opportunity as a learning experience to teach the team what a good pull request flow looks like.

We worked through the code changes (mostly changes in JSON config files), but in my haste and drive to make a good learning experience, I skipped over one key part of the pull request process - I didn't take the time to understand the scope of the work encompassed in the pull request. That key omission ended up causing the disaster that started this post. Buried inside that pull request was a one-line change in a CSS file - the background-size attribute in the main header image class was changed from contain to cover. That change was 100% out of scope for the rest of the involved work and wasn't noted anywhere other than a file diff.

We've had issues with scope management in the past, but one of the unmentioned benefits of a pull request and code review is scope management. Because we're walking through proposed changes and commits, reviewers need to have an understanding of the scope of work represented by the pull request. If the scope of work in the pull request is outside of what's expected, then discussions around the completed work need to happen. The general guideline I've published is to reject the pull request until all of the scope discussions are complete. A rejection isn't permanent, but it provides a clear indicator that discussion needs to happen.

Ideally we'll never have to rely on this to resolve scope issues with ongoing work (we've failed if we let scope management rest here), but if needed, this is the best option we've got.