Rubocopout#

Fri, 04 Dec 2020 22:46:56 +0000

Day 4 of Advent of Blog and already I've run out of things to talk about.

Today at work I pushed a change that caused a production bug. I'd set out (and succeeded) to fix a different production bug, but when I pushed it into CI I got a complaint from Rubocop that I'd exceeded the line count limit for module size and the limit for method size, so I added another couple of changes to see if I could make it shorter.

Which I did, but ... if you don't know Rails this explanation - indeed, most of this blog post - will not make sense, and I apologise in advance. Sorry.

It was a controller. The original code had a before_action that checked if the request method was HEAD and handled it specially by sending a head response. I moved this to the beginning of the handler. Two hours later, we're getting DoubleRenderError reports in production. Reverting the commit fixed the error. I don't know the root cause yet (this was on Friday afternoon) but I speculate that there is a subclass of this controller class that overrides handle to call super but not return immediately. Or something. When the HEAD case was in a before_action this wasn't a problem because Rails knows when a before_action has rendered, and skips the rest of the response processing after that point.

So anyway. I screwed up by making an obviously-correct change that wasn't, and because it is in my nature as a programmer to find some reason it wasn't my fault (narrator: it was his fault) I'm directing my ire at Rubocop - or at least, at my willingness to pay too much attention to it.

From https://www.rubypigeon.com/posts/my-beef-with-rubocop/ which I would like to endorse as good general advice about automated code style warnings in any language:

I don’t dislike RuboCop. It’s well made, and a useful tool.

The problem starts when it is viewed not as a tool, but as a set of divine commandments. Thou shalt not make implicit nils explicit. Thou shalt not write long-form conditionals. Thus saith the RuboCop.

This is not necessarily a problem with RuboCop itself — it’s a problem with how people sometimes use RuboCop. One could argue that the defaults aren’t great, but they are not a problem until someone hooks them up to CI.

and from the author's lobste.rs comment -

I feel link linters pull you towards a certain level of readability. Whether they pull you up or pull you down depends on where you started at.

While I'm here and thinking about Rubocop, and because I don't often talk Ruby in this blog, if you are a Ruby programmer and your team uses Rubocop, do yourselves a favour and change the default setting for Style/BlockDelimiters from line_count_based to semantic. Here is Avdi Grimm on the topic.