5 minute read
Over the course of Shopify's 13-year codebase history, the core platform has never been rewritten. That meant a slew of outdated code styles, piling atop of one another and without a lot of consistency. By 2012, our CEO Tobi created a draft Ruby style guide to keep up with the growth. Unfortunately, it never became embedded in our programming culture and many people didn't even know it existed.
Ruby is known traditionally as a creative language, but our rate of growth necessitated a way to contain the chaos. One of our engineering leads, Volmer Campos Soares, recognized this issue and introduced to Shopify a linter for Ruby called RuboCop. Unfortunately because it would find violations in our entire codebase, and our codebase had never been linted, RuboCop swamped us with violations.
Policial
Our solution was creating Policial, a wrapper built around RuboCop that only runs on pull requests. It would listen to events through GitHub’s webhooks, and whenever a pull request was created it would comment on the lines that contained code violations. A big part of introducing this tool was ensuring developers were onboard. However, right away, our teams expressed how unimpressed they were with the tool. They found it was too noisy and added too many comments to their pull requests. It was still being used, but we knew that we needed to improve the tool.
Improving the Tool
So we went looking for another solution and used the commit status API in GitHub. This tool was great because, instead of commenting multiple times in a pull request, it would change the status of the pull request to green if there were no violations. But if there were violations, it would turn red and they could retrieve the details of the lines that needed to be changed.
This was an opt-in feature and we found that 50% of our devs chose to opt in to it. Along with the aforementioned updates, we also created the Shopify Ruby style guide, greatly extending the initial draft from 2012. It went into more detail about our style preferences and gave more context into the importance of a consistent style guide.
After six months of experimenting Policial in Shopify core we wanted to gauge how developers were feeling about the tool, so we sent out a survey. The results showed that the majority of people rated their experience with Policial as positive. The improved code consistency made reading and maintaining code much more easier, and was very helpful for new developers working on our codebase. So we enabled Policial for everyone, and pull requests in Shopify core would be inspected for code style violations.
Fixing Existing Violations
While Policial could prevent new code style violations to the codebase, it doesn’t help much to fix the existing ones. Fixing all existing violations that piled up on Shopify core’s master branch over the years revealed to be a very challenging task. This issue required some serious planning to avoid disruptions and side effects. We still wanted to tackle this problem so we set out to fix it during one weekend when there would be downtime for merging pull requests.
One of the issues of pushing thousands of line changes is that our names would be listed as a part of the Git history. This meant that if there were any issues with a line of code, people might potentially seek us out as responsible for the code. So we connected to GitHub using a bot, allowing people to tell apart our fixes and actual code changes in the Git history.
Another challenge was to figure out a way to prevent people from re-introducing code style violations that were already fixed. Since Policial is a distinct commit status in pull requests, developers are able to ignore it and only mind about CI’s status. The solution was to enforce RuboCop also in CI, so code style violations would cause CI to fail just like a test failure. We introduced RuboCop in BuildKite’s stack to inspect all core files and enforce all rules that were 100% fixed by us.
Fixing all existing violations is still an ongoing effort. The goal is to reach full compliance with Shopify’s Ruby style guide in core’s master. By then RuboCop could be run in all its glory in CI and an extra tool such as Policial would no longer be needed (at least to inspect Ruby!).
Final Takeaways
Tackling an old codebase that spanned over a decade and affects hundreds of people came with some challenges. Here are some things we kept in mind to keep us on the right track.
- It’s important to discuss whenever you’re thinking of taking this on. Developers are opinionated and would like to discuss things that may affect them.
- Allow for rules to be challenged. If something isn’t working, discuss whether it should be removed or not.
- It’s all about timing, so developers need to be onboard for this to work.
- Don’t give up when it gets hard. We’ve gone through various versions and it’s been tough at times. Keep working at it till you get to where you need to be.
- Have fun during the process!