How to Apply RuboCop to Your Large Rails Codebase

Development

Reading Time: 4 minutes

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.

Sign up for a free Codeship Account

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:

  1. Keep the status quo; don’t let it slip further.
  2. Improve the codebase over time.
  3. 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?

Subscribe via Email

Over 60,000 people from companies like Netflix, Apple, Spotify and O'Reilly are reading our articles.
Subscribe to receive a weekly newsletter with articles around Continuous Integration, Docker, and software development best practices.



We promise that we won't spam you. You can unsubscribe any time.

Join the Discussion

Leave us some comments on what you think about this topic or if you like to add something.

  • Pingback: How to Apply RuboCop to Your Large Rails Codebase | Dinesh Ram Kali.()

  • Any suggestions for an easy hosted solution to this?
    > From the script you can push the current amount of warnings to your favorite tracking tool and visualize the progress over time

    • ben

      Hey,

      we use Librato Metrics for every possible metric that is engineering related. You could push the number of warnings to Librato and create a nice little Chart for the number of warnings and see them going down (hopefully)

      It’s also motivating to get the number down.

      thanks!
      ben

    • iconnor

      We push progress reports via slack and track your velocity rather than absolute linting violations. This article is spot on that tracking your progress going forward rather than past mistakes is the best way to make your code better.

  • Even further upstream from rake tasks and CI, I’ve enjoyed the Overcommit Gem, which allows you to install git pre-commit hooks which run linting on whatever you’re trying to check in.

    It makes commits and other git activities incrementally slower, but can almost eliminate style violations completely.

    Gem is here: https://github.com/brigade/overcommit

    My sample config is here: https://github.com/adarsh/dotfiles/blob/master/.overcommit.yml

    • ben

      Hey Adarsh,

      thanks for the hint! i will check out Overcommit. Maybe we can add it to our mix and prevent another CI roundtrip. i still want to keep rubocop on our CI because it’s very visible to other team members that review the PR.

      thanks again!
      ben

      • Harm-Jan Blok

        Overcommit also has the feature to ‘run pre-commit hook against all tracked files in repository’, which for example might be useful in your CI. Run it with `overcommit -r`. This will verify your codebase for all configured pre-commit hooks, for example: rubocop, scss-lint, haml-lint. Using the `on_fail` and `on_warn` configuration options you can tailor the exit status to fit your current needs.

  • Michelle

    Codeguard is similar but free and less advanced. https://codeguard.herokuapp.com

  • You can also run Rubocop with –auto-gen-config, this way you generate a list of ‘todos’ files that are ignored by rubocop checks.

    • beanieboi

      Hey Gianni,

      thanks for the comment! i will checkout the config file. from my first try i didn’t capture warnings. is there a way to also capture warnings? :)

      • You’re welcome, it’s a pleasure
        I just tested on a project, I got the ‘Lint/UselessAssignment’ in the .rubocop_todo.yml which is a warning. Is this what you’re looking for?