if foo? then
blah
end
will result in a complaint about how one should remove the "then". Sure, you can configure rubocop to not make that complaint, and then the next one, and then next one ... but whatever happened to convention-over-configuration? I choose the convention of not using rubocop.TIL (something not that useful)
But most of my Ruby projects are tiny tools with a bus factor of 1. I find "rufo" as a minimal formatter quite nice for those (there are VS Code plugins). For the most part it normalizes '' to "" in code that I might have copied from somewhere else, but doesn't go on to lecture me that "if not" must be written as "unless", and all the other things that the Rails community cares about.
def foo(x)
self.bar = x
end
and complained that `self.` should be removed. Somebody ran rubocop with autofix. It changed the code to just `bar = x`, which is not the same thing (it just creates a new variable called bar), and it resulted in some really horrible bugs that made their way to production.I never used rubocop again.
(I'm really hoping this was just a rubocop bug, and has since been fixed, but it's enough to ruin your trust.)
if some_verbose_condition && some_other_verbose_condition
do_the_thing unless excluded_case || other_excluded_case
end
Rubocop changed this to if (some_verbose_condition && some_other_verbose_condition) && !(excluded_case || other_excluded_case)
do_the_thing
end
which is just worseand then it had the gall to complain that the line containing the `if` was too long.
That said, if you disable half its rules, Rubocop can be a useful tool. We've long had a list of database migration best practices, which we've built up over the years to ensure changes to our application's database schema don't cause downtime or other issues. Lately I've been writing cops to automate checks against these practices.
Useful feedback: "Heads up: changing the type of that column is going to lock the users table and bring the site down; see $BEST_PRACTICES_DOCUMENT"
Not useful feedback: "zomg ur cyclomatic complexity si 2 high!!1"
These would be great to share and popularize. Too many Rails shops do this badly!
Prettier does a fantastic job of this for many languages. RuboCop is of course still useful for catching things that impact logic/functionality/performance (e.g. the issue presented in the article), but it's not a great choice for enforcing code formatting since it is far too configurable.
We only lint the files that changed after the date we integrated rubocop : `git -c log.showRoot=false log --no-merges --pretty=format: --name-only --since="2022-01-01"`
Then we heavily customized `.rubocop.yml` to avoid the rules that were not auto-correctable.
It was still brutal for a couple of weeks but now, maybe 2 years later, everything is fine.
In particular, a lot from the lightning talk resonates with me.
- they bring example execution order into play, especially with nested contexts and nested `let`s shadowing other above.
- they invite DRYing up test code, making it really easy to couple unrelated tests together and hard to understand tests in isolation.
- the corollary to the above is creating a brittle test suite. (in any case if DRY is a footgun in production code, it's doubly so in test code.)
- they require you to divert your attention from the examples to see what the states of your test objects are going to be.
These pitfalls can be avoided if, for example, you favor building up your test object graphs inside your examples.
(@r-s That's not the same thing though, they're talking about the eagerly evaluated version of `let`.)
I can tell you why I do. I first encountered the term 15 years ago or so when studying the medical literature on HIV/AIDS. At the time (might still be this way) the most effective treatment was the now famous "drug cocktail", by applying multiple drugs that were individually only moderately effective we found that HIV/AIDS patients could live a somewhat normal life. In fact the treatment worked so well that after a decade of treatment some people live the rest of their lives without any detectable viral load at all, they are in effect cured of the disease and no longer needed treatment. This is the best practice, as it results in the best outcomes statistically speaking. The life expectancy of HIV/AIDS patients went from a few short months after infection to on par with the general population. This was provable, and not really a matter of serious debate as the evidence is overwhelming.
The formatting of a line of code one way or another feels completely different than that. It feels like somebody with a blog prefers it that way. It really should be called "best preferences" or something.
I prefer 'good practices' or 'guidelines' but as far as something like Rubocop is concerned, I don't really agree that its default setup meets that standard. Without some careful tweaking of the configuration you're likely to end up with a codebase full of premature abstractions that exist for literally no other reason except to satisfy Rubocop.
There is a subset of Rubocop rules that does a much better job, in terms of identifying potential sources of bugs (e.g. calling non-TZ aware date objects) and replacing deprecated methods with their alternatives where possible. The tool is worth it for that, so long as you disable all the nonsense about method lengths, class lengths, number of methods in a class, etc.
That's a great example. What if I'm working in a embedded system with limited memory and I need to shave off a few kilobytes? What if time zones don't matter for my implementation, say I make a timer app and the only thing that matters is the delta between two times?
There are things that I think rise close to the level of best practices. For example your password hash comparison function should probably run in constant time, but a linter is never going to pick up on something like that.