Sprint retrospective is the mechanism that enables us to hold ourselves accountable to the standards we set as a team. It's responsible for helping us evaulate our processes as well as catalyzing culture change.
Our sprint retrospective philosophy is to be open to new ideas and be willing to experiment. This ensures our processes evolve alongside the growth of the team and company. An example byproduct of our sprint retrospectives is our code review process. We were trying to determine how we could cut down the number of bugs that made it into production. Were we not adding enough tests? Should we introduce a formal code review? The latter's discussion resulted in a divided team.
One camp was comprised of engineers who had worked at larger companies that had instituted formal code review processes. The other camp consisted of engineers who had only worked at smaller start-ups (fewer than 10 engineers) where code review had never been formalized.
The first camp was haunted by memories of day-long code review sessions and was skeptical that any code review process would be productive for the team and an efficient use of their time. The second camp was eager to prove the first camp wrong. My job was to facilitate this discussion in a way that would yield a solution that both camps could agree on without reducing engineering throughput.
The framework we used to shape the process was the following:
- The process should be lightweight - code review should be an efficient use of an engineer's time.
- The process should be transparent - code review should be visible to the entire engineering team.
- The process should be consistent - effective code review should be reproducible by any developer regardless of position or experience.
We use GitHub pull requests to accomplish this. If you're unfamiliar to pull requests, watch Zach Holman's presentation on How GitHub Uses GitHub to Build GitHub. We use pull requests because:
- It facilitates the discussion for the feature, the code, and the code review.
- It's living documentation that can be referenced at any time because it houses the backstory, strategy, and stream of commits behind any feature.
- It allows code review to be done remotely and asynchronously.
- It allows you to easily reference other code, commits, or pull requests.
How it Works
We always require the approval of at least two people before signing off on a pull request. One developer has the 
role and the other has the 
role.
The role is responsible for throughly reviewing the code through manual testing on their local machines or on a staging server. It's their job to ensure the code behaves as intended (no bugs, intuitive UX, etc.).
The tester is responsible for the visual check of the code and is encouraged to be critical of the implementation, the design of the code, and the unit/integration tests. Essentially it's their job to ensure the code quality meets our team's standards (as well as catch the occasional typo).
Our Guidelines
- If the tester gives feedback on the code, they should attempt to do so in a way that educates the author and the rest of the team. For example, instead of saying 'this SQL call is inefficient, fix it' the tester should provide a better way to do it and explain the inefficiency of the current code so that others can learn from the example.
- As long as feedback is given to prevent bugs, improve code quality, or provide education, there's no reason to take any critical feedback personally.
- The pull request should have a thoughtful Readme with a detailed background so every person has context for the code review.
- If it's not obvious how the should manually test the code, it's the author's responsibility for providing testing steps.
- The author should have already tested all the steps they listed so that the tester isn't wasting their time running into bugs that could have been easily discovered.
- If any seed data or setup is required to do the testing, the author is responsible for providing rake tasks or steps to put the branch in a place where it can be easily tested.
- The test suite should be green before the pull request can be reviewed.
- The author should have already reviewed Code Climate for their branch to ensure no obvious code quality errors were made.
- If the author performed all the testing in one browser (ex: Chrome), the author should suggest that the tester performs their testing in another browser such as Internet Explorer to prevent cross-browser compatibility bugs.
- The and roles should be clearly defined at the top of the pull request so every developer knows who is responsible for reviewing each pull request.
Results
It took us close to two years to arrive at this exact version of the process, but it's turned into one of our most beloved. Even though we have more engineers, more servers, and more traffic to our website than ever before, we currently have fewer known bugs in production than we did when I started at thredUP in 2010. Surprisingly, there were other intrinsic benefits to this process:
- Fewer bugs going into production means we're not dropping what we're working on as often to firefight, which results in less context switching throughout the day.
- All of the Readmes and discussions from our pull requests have provided us with helpful documentation for our code base. When a new hire wants to learn about system X, we just point them to system X pull requests.
- Our code quality (according to Code Climate) has never been better.
- Since each pull request has at least 3 engineers on it, we no longer have an issue with domain knowledge hoarding where only one developer knows certain parts of the app. If a bug does pop up, there are at least 3 people that should be able to resolve it.
- The feedback and conversations that have taken place during code review have become valuable learning opportunities for every level of engineer on the team.
