00:00:02.240
Good code review can be a fun, positive, and enriching experience. It helps you establish a clear review program that can significantly benefit your current developers and potentially attract new talent.
00:00:09.920
That's certainly what we experienced at Powershop.
00:00:24.080
Hello, I'm Tong. Here are some ways you can reach me after this talk. I'm currently working as a team leader at Powershop.
00:00:36.079
Powershop's mission is to help our customers better understand their power consumption. We achieve this by giving them better control over their power usage through various means, such as straightforward understanding of their meter readings and providing online bills in an easy and fast way.
00:01:01.920
To facilitate all this, we have a single monolithic Rails application that powers almost all areas of our business. This includes our online shop, the call center, and handling file exchanges with other parties in the industry.
00:01:36.240
During this talk, I will discuss how my team improved our current review process. I want to emphasize that this talk will focus more on the cultural aspects, as I believe that if we get people and practices (also referred to as culture) right, then the technical aspects will naturally follow.
00:01:44.400
There are plenty of blog posts and videos about technical checklists for good code reviews, but the cultural side often receives less attention. Therefore, I want to highlight that today.
00:02:10.319
More importantly, it's about transitioning from a state of panic, exemplified by feeling overwhelmed, to a place where we have a happy team that can efficiently ship productive code.
00:02:31.840
Around April 2015, our team faced significant challenges. We were in the middle of a large project, and in the last nine sprints, we failed to complete our tasks on time. This was quite disheartening as it left us feeling deflated.
00:02:41.760
We struggled with a series of extensive changes, essentially refactoring half the application. This involved engaging in multiple review cycles, some with external reviewers, and we received substantial feedback along with extensive review notes, which we found challenging to manage.
00:03:27.200
These large changes required intricate code adjustments that were risky. Initially, we spent excessive time reviewing, especially as the stakes were so high. During that period, the average size for a code change was around 1,000 to 2,000 lines of code.
00:03:54.639
Getting a 2,000-line code review at the end of the sprint, with only a day to address it, was particularly stressful. This was compounded by external testing pressures. We were not completing our sprints on time, and this slow pace meant that our team was only able to release new code to production about once a month.
00:04:34.320
Releasing code to production that involved large and risky changes carried inherent risks, as encountering issues was nearly inevitable. We discovered this during one of our production deployments, which thankfully did not result in data loss.
00:05:18.240
In the retrospective for sprint 119, our team resolved not to endure such chaotic processes again. We decided to adjust how and when we reviewed code, opting to conduct reviews more frequently— not just at the end of tasks.
00:05:36.640
This entailed breaking changes down into smaller increments to make them more manageable and minimizing the necessity for massive changes that might prove unnecessary.
00:06:01.680
Another change was encouraging everyone to participate in the review process. We’ll delve deeper into this shortly.
00:06:57.919
Before I outline what we changed in our code review process, I want to highlight what we were already doing well. First, we had made code reviews mandatory, which is not the case in every organization.
00:07:22.720
Having mandatory code reviews has proven invaluable. This practice has helped catch numerous issues that might otherwise go unnoticed across our seven years of conducting reviews at Powershop.
00:07:40.560
Secondly, we established a definition of done; this is crucial because it prevents unnecessary suggestions and avoids prolonged feedback on trivial matters. If a review highlights a minor improvement and it doesn’t align with our guidelines, we can defer it.
00:08:12.320
Thirdly, we emphasize testing our code. Having specifications in place is slightly more critical than the review process itself in terms of safeguarding us against bugs. The number of embarrassing issues we've caught through our testing has been impressive.
00:08:50.240
Thus, the first change we decided to implement was to increase the frequency of reviews. Previously, we only conducted reviews at the end of the coding process, which created a significant bottleneck.
00:09:02.720
To address this, we resolved to review code after completing every task. For instance, if a change comprised three tasks, such as writing the model, creating the service, and integrating everything into a controller, we would review at the end of each of those tasks.
00:09:54.800
We found this approach worked well, as it allowed for timely feedback. If there were issues, we could address them immediately instead of waiting until a larger review at the sprint's conclusion.
00:10:06.800
Initially, we attempted to review after every commit, but this proved overwhelming due to the number of commits. Eventually, we returned to task-based reviews, which yielded better results.
00:10:30.839
Another significant change was ensuring that everyone in the team remained aware of each other's tasks. Before this, we often divided into smaller groups working on separate changes, only to reconvene for reviews two weeks later, which wasted time.
00:11:12.720
To optimize this, during the early stages of a sprint, we would hold a planning session to outline the high-level tasks. This small adjustment allowed team members to familiarize themselves with changes, improving the speed of our reviews.
00:12:01.199
To recap, we now plan out work collectively, encourage frequent feedback after every task, and maintain a final review before merging into the master branch. However, frequent reviews alone didn’t fully resolve our issues regarding large code reviews.
00:12:32.080
We subsequently engaged with our product owner to break down changes into smaller, more manageable independent chunks. This process benefited everyone: the product owner could see incremental progress, and testers had more code to work with instead of waiting indefinitely for us to complete everything.
00:13:00.000
Additionally, we recognized that a limited group touched the code reviews, usually involving me and one or two senior developers. Through discussions, we decided that everyone should take part in the reviewing process. This fostered an immersive learning environment, particularly for new team members.
00:13:45.680
Explaining the mechanics of reviewing and offering them opportunities to engage, even if they were new, proved beneficial. I’ve seen junior members give incredible feedback and catch mistakes that even the more seasoned developers missed.
00:14:09.840
For instance, I had a junior reviewer identify numerous bugs in my code, which was impressive. Such experiences demonstrate that even newer developers bring valuable perspectives to the reviewing process.
00:14:31.200
Sure, minor bugs slipped through occasionally, but the trade-off was worth it. A quicker feedback cycle allowed new developers to learn rapidly, while we still managed to catch major issues before production.
00:15:14.000
Additionally, we retained a final review point before merging to master. In the next team I joined, we adopted a collaborative review model where each member would review each other's code, resulting in more comprehensive feedback.
00:16:03.680
Moreover, reviewers have the option to seek second opinions from outside team members, which we affectionately termed 'second breakfast'— a nod to The Hobbit. This practice is particularly useful when dealing with complex code involving finance or data integrity.
00:16:32.320
Furthermore, our team of testers plays an essential role in quality assurance, helping validate code changes. The release manager also conducts quick reviews to ensure nothing might disrupt production.
00:17:11.040
Six months later, the changes we made resulted in significant improvements in our sprint performance. The team has been consistently happier as we achieve satisfactory outcomes time and again.
00:17:25.160
Other teams began adopting our processes, which has been flattering as well as beneficial to their own projects. Powershop has also seen tremendous growth, expanding from 30 to 50 developers and evolving from three to seven teams.
00:17:46.640
To summarize, effective code reviews can provide an amazing experience for both current and future team members. I encourage all of you to reflect on ways to improve your teams to enhance their workflow and, ultimately, foster a more productive and happy environment.
00:18:06.370
Thank you.
