Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add rubocop-disable_syntax #11513

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft

Conversation

mitchellhenke
Copy link
Contributor

🛠 Summary of changes

This is a proof-of-concept intended to start discussion about changing some of our Rubocop rules

.rubocop.yml Outdated

Style/DisableSyntax:
DisableSyntax:
- unless
Copy link
Contributor

Choose a reason for hiding this comment

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

YES

Copy link
Member

Choose a reason for hiding this comment

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

I don't actually mind unless, but I don't oppose this change either.

I was warned at a previous job that "people will hurt you" when I wrote an unless... else though. 😉 (unless !something is also cruel.)

Copy link
Member

Choose a reason for hiding this comment

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

(The size of this PR seems to argue that it's well-entrenched and should maybe stay too?)

.rubocop.yml Outdated Show resolved Hide resolved
Comment on lines -230 to +234
Description: Limit lines to 100 characters.
Description: Limit lines to 120 characters.
StyleGuide: https://github.com/bbatsov/ruby-style-guide#80-character-limits
Enabled: true
Max: 100
Max: 120
Copy link
Contributor

Choose a reason for hiding this comment

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

I am happy enough with 100. But I wanted to point out we do have our JS files with a matching 100, if we change this I think we should change them together to keep the whole codebase consistent

"printWidth": 100,

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.

4 participants