Talks
Keeping Code Style Sanity in a 10-year-old Codebase
Summarized using AI
Keeping Code Style Sanity in a 10-year-old Codebase
by Gabi StefaniniIn the talk "Keeping Code Style Sanity in a 10-year-old Codebase" by Gabi Stefanini at RailsConf 2017, the challenges and strategies associated with maintaining code consistency in Shopify's decade-old Ruby codebase are explored. The discussion highlights the developers' diverse opinions on code style consistency and the complexities that arise from enforcing a uniform style across a large and diverse team. This session reveals how, over the years, the codebase grew to more than 700,000 lines of Ruby code and involved contributions from over 700 developers.
Key points discussed include:
- Historical Context: Shopify's codebase has evolved since its inception in 2004, originally lacking consistent styling guidelines.
- Initial Attempt at Consistency: In 2007, a rudimentary style guide was introduced, which largely fell by the wayside due to the slow adoption of consistency policies.
- Introduction of Robocop: In 2014, the Robocop gem was integrated for basic style checks; however, it faced backlash for overwhelming the codebase with violations due to its strict checks.
- Policia Tool Development: To mitigate the influx of style violations, the Policia tool was developed, which checks new and modified code, allowing the team to focus on code quality without disrupting existing code.
- Survey Feedback: In 2016, developer feedback indicated mixed feelings about Policia, praising its advantages for onboarding and readability but critiquing the time needed to address style violations in PRs.
- Evolving Strategy: The team transitioned to a new approach where they would start with zero configured rules in Robocop, allowing them to add only the relevant ones, improving receptivity among developers.
- Iterative Improvements: Continuous improvements were noted, and the team aimed to integrate style checks as part of the CI process, which would streamline the developer's workflow and code adherence without overburdening them.
The session concludes by emphasizing the importance of discussion, adaptability, and incremental changes in establishing a consistent coding standard. Key takeaways include:
- Developers should have a forum for discussing code style rules.
- Customizable linting rules should align with specific team needs.
- Flexibility in adapting rules can lead to an easier acceptance among team members.
- Reinforcing the understanding of coding standards improves code readability and developer onboarding, ultimately enhancing productivity.
Overall, Gabi's talk provides valuable insights into balancing code consistency with developer autonomy, reflecting on the trials and tribulations encountered at Shopify.
00:00:12.230
Hi everyone, so let's get started with your last session of the day before the big keynote. My name is Gabi and I am a developer at the Get Done at Scale team at Shopify. This week is kind of special for me because I'm celebrating quite a few things. Today, I'm completing one year at Shopify, marking my one-year anniversary as a Ruby programmer. It's also my very first time at a conference, and it's the first time I'm speaking to a technical audience. This basically means that I'm terrified and if I speak way too fast, the worst that could happen is that you will be out of here 20 minutes early, so please don't kill me.
00:00:31.019
Before we start the talk, I'd like to clarify a couple of things and address the elephant in the room. I am NOT here to convince you that you should implement code style consistency throughout your entire codebase. So, you may wonder, what am I here to discuss? I will share the history of our journey to tackle this issue at Shopify. We will look at things that worked, things that didn't, and some tools we're using. Furthermore, I hope to provide you with insights on how to foresee issues before they arise when tackling this problem in your own codebase.
00:01:41.159
Let’s go back to the beginning. If you have seen the talks from my colleagues Simon and Raphael, you probably have a bit of knowledge about where it all began. While researching for this talk, I came across our first commit ever on GitHub and I took a screenshot so you can see it as well. It was made by our CEO and founder, Toby, and it dates back to 2005. However, I found out that our codebase actually existed before that; it started in 2004 but was using SVN instead of Git. Over the course of these 13 years, we have noticed two significant things. Firstly, we have an immense amount of lines of code in our base. Secondly, we have a lot of developers—our team today consists of over 400 Rails developers working primarily on our core codebase.
00:02:49.709
This is what our repository looks like after 12 years. We had contributions from over 700 people across different departments, including developers, content writers, and more. We are still growing. So, what does this mean for code style consistency? You've probably heard that Shopify is one of the largest Ruby on Rails shops in the world, which means we face some unique problems in our day-to-day. We have to develop creative solutions to maintain consistency in our 700,000 lines of Ruby code.
00:03:40.739
The first question we had to ask ourselves over these years was whether we wanted to make our codebase consistent. Imposing a single code style actually goes against the intentions behind the Ruby language. I came across an interview with Matz, the designer of Ruby, where he mentions that Ruby is supposed to be a fun language to work with—one that allows the user to choose what works best for them, unlike other languages that dictate only one way to accomplish tasks. Ruby is supposed to encourage creativity. However, Ruby has evolved significantly over the years. Some styles that used to be common are no longer in use. For instance, let’s look at the updated hash rocket syntax, which has largely been replaced by the newer hash syntax. Both styles work, but this brings me to my next point: Shopify has never undergone a complete rewrite in these past 13 years, so all these outdated styles remain in our codebase.
00:05:00.960
This situation brings up issues where we have old-fashioned code littered throughout, creating inconsistencies. Let’s dive into some history here. Our codebase was born in 2004 using a new framework with just a couple of developers working on it. No one was really concerned about code style consistency at that time. Then came 2006 and 2007, I’m not going to do this for 40 minutes, but you get the point. By 2012, we had a significantly larger team, and our codebase was growing. So, Toby, our CEO, created a draft style guide for the Ruby language in our codebase. He provided a handful of rules to guide our developers on how to write code, focusing on white spaces and other conventions.
00:06:21.080
Things got busy, and the issue of code style was set aside until September 2014. At that point, we introduced the RuboCop gem to our codebase to enforce basic code style rules. RuboCop, an open-source static code analyzer, serves as a linter that follows many of the community's code style conventions for Ruby. The intention was sound, but our large, ten-year-old codebase had never been linted, which led to many violations and left us overwhelmed. Thus, we had to disable several cops because there were just too many issues to address all at once. Instead of running RuboCop through the entire codebase, we decided to find a temporary solution, focusing on the lines of code that were being changed or added.
00:07:49.590
This is where our PoliceShow gem came into play. Policia—or Policía, as we'd say in Portuguese—is a wrapper built around RuboCop that only runs on pull requests and doesn’t affect direct commits on the master branch. It identifies code style violations only in the lines that were modified or newly added. It was inspired by the Hound tool made by Thoughtbot but adapted to meet Shopify's specific needs and constraints. Policia utilized GitHub webhooks, and every time a pull request was created, it would comment on the PR, highlighting code style violations introduced. Initial feedback was mixed; while some developers found it helpful, many complained that it cluttered PRs with numerous comments, which was a common complaint I heard while gathering feedback.
00:09:20.439
Regardless of the noise, the team continued using it, but it became clear that we needed much-needed improvements to our tools. October 2015 was a pivotal month when significant changes were made. We explored using the GitHub commit status API instead of commenting all over the PRs to convey code style violations. This meant that the Policia status would change color depending on whether violations were found in the PR. If no violations surfaced, it would show green, indicating a merge could proceed, while red would signal issues that required attention before merging. This transition drastically improved usability.
00:10:52.340
This is how our Policia tool looked. When users clicked the details button, they could view all style violations, including the lines where they occurred and explanations of the violations. Back then, Policia was an opt-in feature that developers could voluntarily sign up for, allowing them to have their pull requests checked by this tool. Even though the tool wasn't perfect, we witnessed a great adoption rate and by early 2016, over 50% of our developers had opted in. Another important addition during this period was our Shopify Ruby style guide, which was launched to help developers understand what code violations were prevalent and how to fix them.
00:12:01.360
The style guide set clear expectations for our Ruby developers, defining acceptable code styles and encouraging them to take ownership of their code. It also included numerous examples of correct and incorrect code styles, which helped eliminate the guesswork when writing code. In March 2016, we sent out a survey to all our developers to gauge the good, the bad, and the ugly regarding Policia and the new style guide we had implemented. Some questions included whether they believed Policia played a positive role in pull requests. The majority stated it did, appreciating its role in onboarding new developers as it made the code more consistent and allowed code reviews to focus on the code itself rather than minor formatting issues.
00:13:30.350
However, there were also concerns. Developers expressed that pull requests took longer to complete since they had to address all violations after they pushed their code to GitHub and seek feedback from their reviewers. They found this process cumbersome. Additionally, some mentioned that these changes sometimes made files less consistent, thus reducing readability. An example of this occurred with the hash rocket syntax; if part of the same file was old code while another part was newly added, it caused confusion, especially for new developers trying to learn Ruby and Rails.
00:14:43.330
When asking our developers whether they learned anything valuable through Policia, most replied affirmatively. They reported learning about syntax improvements and best practices in the Ruby community, which made it easier for them to write code, especially newcomers from non-Ruby backgrounds. However, there was also negative feedback from experienced developers who felt the tool was unnecessary as they already knew the conventions and could make those decisions themselves. They were also frustrated with the tool's lack of insight into why certain rules existed and how they benefitted our codebase.
00:15:59.600
When asked if Policia made their lives easier, most said yes because it made the codebase more predictable and reduced the mental overhead of remembering formatting standards. Some developers, however, felt it increased the time spent on their coding tasks as they had to fix violations, and at times the rules seemed unclear. Another recurring theme was that different projects did not consistently apply RuboCop, which led to the inconsistency in code style across the developer teams. One of the most interesting pieces of feedback we received was to have Policia enforced across all projects at Shopify. This was not merely a matter of opinion; they were real developers directly using these tools and sharing their experiences.
00:17:18.280
With such a great response, we decided to activate Policia for all pull requests opened in the Shopify codebase. Other apps could also sign up for the service and include it in their projects. Importantly, Policia was configured so that developers wouldn't fail their builds even if code style violations were present in their pull requests. This was beneficial because it meant that we could still merge emergency PRs quickly, even if they contained some violations.
00:18:14.640
As we became more familiar with Policia and RuboCop, we took steps to enhance our development process. We included running RuboCop in our CI, starting with one component, primarily in our checkout files. This way, RuboCop would flag violations, and your build would fail if you introduced a new violation. We also developed custom cops to address issues that weren't strictly code style related, such as banning previously deprecation code from being added in new files. This improvement made a significant impact.
00:19:38.990
After positive feedback from the checkout team, we prepared to roll out RuboCop across our entire codebase to identify and fix all existing violations. Our plan included running all correctable cops throughout the codebase, which worked well. Next, we needed to determine the best time to merge those pull requests, focusing on minimizing potential conflicts during peak development times. The weekend was considered ideal since fewer developers would be actively coding.
00:20:53.580
Despite our preparations, we faced major blockers that hindered our execution. The main issue was that running all fix-up commits would obfuscate our commit history, complicating blame assignment. This became problematic because we value ownership at Shopify, where each developer should feel responsible for the code they touch. We also learned that a week at Shopify represents considerable activity since we were deploying thirty times a day, making it challenging to incorporate bulk changes without causing significant conflicts.
00:21:51.790
As a result, we decided to close those pull requests and the related tracking issue, with our existing problems still intact. While Policia operated effectively and our code was becoming more consistent, the tools we employed weren't truly making developers' lives easier. Feedback revealed that we were enforcing rules that were either unclear or seemed unnecessary. Initially, all RuboCop cops were enabled by default, but we later decided to change our strategy to begin with zero enabled cops and only add those that we deemed essential.
00:23:04.450
This new approach fostered discussions on why new cops should be added to our codebase while removing unnecessary ones. By January of this year, we introduced an editor feature to help developers track the history of specific lines of code, which became invaluable. As months passed and our new strategy for cops began to show results, our developers felt increasingly receptive to the idea of having a code style guide, feeling confident that improvements were being made, even if the journey had yet to be perfect.
00:24:36.140
We decided it was time to use RuboCop's autocorrect feature across our entire codebase. We used a bot for merging PRs so no one would be overloaded with noise in Slack anymore. By integrating this autocorrect feature into our internal development tool, developers could resolve violations before pushing their code, streamlining the process significantly. Furthermore, we integrated these tools into our API and updated our test suite to ensure it would fail whenever violations were introduced.
00:25:50.230
Unfortunately, we encountered a few significant challenges. One major issue was that linking the entire codebase to the test suite caused the process to take considerably longer than our slowest test. This feedback made it clear we had work to do to improve developer satisfaction. We identified that large files, such as migration files, were partly to blame, which led to our decision to exclude those files from being linted to improve execution time. However, we acknowledged that this was merely a temporary fix.
00:27:09.540
As of this past April, several of our codebase components, like checkout, customers, and gift guides, have been fully linted and are now subject to checks during the build process. I'd like to take a moment to appreciate the teams who tirelessly persevered during this phase. Their feedback was instrumental in helping us refine our tools and processes.
00:28:46.300
Today, we are in a better position, but we acknowledged the need to resolve issues related to the multiple RuboCop YAML files we currently have. Moving forward, our goal is to consolidate those into one single file and ensure that all violations are rectified. Developers currently voice their frustrations over having to wait for all checks to run, which prolongs their workflow. To mitigate this, we intend to have the autocorrect feature enabled by default, automatically addressing violations as they arise, allowing tests to be run seamlessly.
00:30:01.120
While we've made significant strides, we still grapple with Ruby linters taking excessive time, and we're exploring effective caching solutions to overcome this, particularly as our codebase continues to grow. It’s important for us to push improvements upstream for the community, enhancing the tools that everyone uses, and critically, we must continuously refine our style guide to provide developers with more context about why specific rules exist and how they contribute to our overall code quality.
00:31:02.360
Let me share some final takeaways from this 13-year journey. The most important insight is that you need to create a space for discussion in your organization about code style efforts, as developers can be very opinionated. Different individuals might have differing views about what code style is best, so having a platform where these discussions can happen is crucial.
00:31:17.160
Secondly, pick styles that will work for your codebase. RuboCop offers a lot of flexibility in terms of coding style preferences; you can adopt one of the many provided or collaborate within your team to determine the most suitable one for your context. Allow for rules to be examined and challenged; just because a rule exists doesn’t mean it should be a permanent fixture. Lastly, be prepared for the effort—getting everyone on board and understanding the changes takes time. It's a challenging journey, but choosing a sufficient timeframe to adapt is key. By staying engaged with your tools and pursuing a step-by-step approach, you can foster an enjoyable process.
00:32:36.450
I've learned a lot while addressing this issue at Shopify, and I've had the opportunity to connect with incredible people throughout this journey. Thank you so much for your time!
Explore all talks recorded at RailsConf 2017
+105