How to Apply RuboCop to Your Large Rails Codebase
As your company grows, your engineering team will tend to fluctuate. With team members coming and going, the style of the code can greatly alter at any given time. But throughout all these changes, your codebase stays; it's not going away. So, obviously, you have to maintain it.
There are various tools out there that will help you achieve that goal. At Codeship, we really like HoundCI, a continuous integration tool for code style. In this post, I’ll explain how we use HoundCI and RuboCop at Codeship and how we introduced it to our team.
How Does HoundCI Work?
First, let’s go over how HoundCI works just to get everybody on the same page. HoundCI watches your PRs and checks the code against your preferred style guide. HoundCI will only comment on files that got changed within the PR. This provides feedback for the files you actually touch and not for the whole codebase.
If HoundCI finds style guide violations, it will comment on the PR, describing which guideline you violated. You still can decide whether or not you want to fix the violations, but that would sort of defeat the purpose of using HoundCI in the first place.
Under the hood, HoundCI uses a variety of open-source tools for linting your code. For example, it will use RuboCop to check your Ruby code and SCSS-Lint for SCSS and so on. You can get an overview of the supported languages in their documentation.
Using HoundCI’s Default Configuration
When we first signed up for HoundCI, we took the default configuration files and tweaked them a little. Really just minor changes that we favored over HoundCI's suggestions. Done, nice.
HoundCI was watching our PRs around the clock. And it commented a lot on them. People started arguing that they only moved code around but hadn’t actually changed it. Or they had just fixed a typo and suddenly they would have to fix the the whole file. There was a constant debate around whether changes could be just merged since they didn't make the codebase worse. This happened a lot in the first week, and it got to the point where we disabled HoundCI.
Trying Again with RuboCop
I thought about our first approach with HoundCI for a while. I still really liked the idea of having somebody neutral to watch our code style. But we needed a different approach to get HoundCI up and running.
I started playing around with RuboCop (without HoundCI) and saw that our codebase had around 900 warnings. We had allowed our codebase to slip, so the number of warnings grew over time.
As a result, we determined that there were a few goals we wanted to reach:
Keep the status quo; don't let it slip further.
Improve the codebase over time.
Promote boyscouting; fix one linter issue at a time
These goals are all actually very different from each other and required different solutions.
When we first tried to use HoundCI, we followed the HoundCI suggestion:
Can I have Hound check my entire repository for style?
No, and we don't recommend doing so. Our rule is "don't rewrite existing code to follow the style guide.”
Sounds reasonable. Why should I spend weeks fixing all the issues? We can do that over time! But as I mentioned earlier, this approach didn't go well for us.
Since then, we’ve integrated all the linters that HoundCI uses under the hood without actually using HoundCI. Our goal is still to use HoundCI in the future; we just have to take a little detour to make it usable for us first.
When I added RuboCop to our codebase, I recorded the number of warnings we currently have. We added a little script to our CI process that will fail the build when we exceed that recorded limit. In this way, we could meet all of our goals. We can ensure that we don't let the codebase slip any further, without annoying people about warnings they didn't produce. It also allows us to improve our codebase over time, rather than via one focused effort.
# !/bin/bash set -e ALLOWED_WARNINGS=546 warnings=`rubocop --format simple | grep "offenses detected" | cut -d " " -f4` if [ $warnings -gt $ALLOWED_WARNINGS ] then echo -e "allowed warnings $ALLOWED_WARNINGS" echo -e "actual warnings $warnings" echo -e "Too many rubocop warnings" echo -e "Try running 'bin/check_rubocop_against_master'" exit 1 else echo $warnings/$ALLOWED_WARNINGS is close enough ಠ_ಠ exit 0 fi
Tracking the Ratchet
It’s good to track your progress. From the script you can push the current amount of warnings to your favorite tracking tool and visualize the progress over time. It's always good to have some nice looking charts to motivate the team and prevent stagnation from taking over.
Where Are We Now?
We are now able to tackle the warnings incrementally; we don't need fix them all at once. We open PRs that are smaller and fix a few warnings here and there. With this approach, we’ve already cut our warnings in half in just about two weeks! This is really awesome.
Our goal is to reach 0 warnings and enable HoundCI again to comment on the PRs. By having 0 warnings, HoundCI will only comment on the violations that we newly introduce. That’ll reduce the noise a lot. No developer likes a high signal to noise ratio.
Are you using HoundCI or any other linting tools as part of your CI process? If so, what challenges did you face introducing them to the team?
Stay up to date
We'll never share your email address and you can opt out at any time, we promise.