Skip to content

Conversation

joshsaintjacque
Copy link

Based on the staff engineering discussion earlier this week, it sounds like we like the idea of removing any rules that could be enforced with a linter or which do not reflect our code base. I suspect a good rule of thumb here is is this rule worth memorizing and nudging in code review? If not, I suggest we lint if desired and toss the rule.

This pull request suggests removing most of the rules in the ruby section since they are lintable and, in some cases, we don't seem to employ them. Wide open to any feedback, especially if you see any rules in here you feel really strongly about.

Once we agree on which rules to remove from this guide, I believe the next step would be to go deeper on the remaining ones by creating dedicated pages with examples for them.

Copy link

@chadellison chadellison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes sense to me. A lot of these are already enforced by the linter.

Here are a few that aren't that we could consider adding a linter rule for:
Avoid organizational comments ('# Validations').
Avoid ternary operators ('boolean ? true : false'). Use multi-line 'if' instead to emphasize code branches.
Avoid bang (!) method names. Prefer descriptive names.

@jeromedalbert
Copy link
Contributor

Nice. Our linter configuration does not seem to currently enforce Order class methods above instance methods, but there is a Rubocop rule for that if we want to enable it. There are a few other cops (like Layout/ClassStructure) related to ordering things in a defined order if we want to automate things even more.

Copy link
Contributor

@avealle avealle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are all of these rules covered by existing rubocop rules we employ today? I think bundling together rules to be removed because they are already covered by active rubocop makes sense. We shouldn't really have anything to discuss.

I do think it's worth having a per-rule PR for things that are going away completely, or that are being transitioned from written rule to rubocop. Also cc:@rayfaddis who mentioned wanting to be involved on reviews where we are removing content from the guide that doesn't have an equivalent in rubocop.

@joshmfrankel
Copy link
Contributor

100% on board with code enforcement via linters whenever possible. Leave documentation for things that can't be enforced with Rubocop, ESlint, etc..

@tute
Copy link
Member

tute commented Jul 2, 2025

Cursor is a good ally at mapping README recommendations with current (or potential) rubocop automated checks. I started a simple PR removing 6 suggestions from ruby and 3 from rails, verifying and mentioning what rules we have that cover for them: #120.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants