Skip to content

non-production updating rubocop hash and array configurations #80

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

non-production updating rubocop hash and array configurations #80

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions ruby/.rubocop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,36 @@ AllCops:
- !ruby/regexp /old_and_unused\.rb$/
NewCops: enable

# Configure cops for styles that we enforce and autocorrect for (where applicable)
# Please add new configurations in alphabetical order and make sure their keys are not re-used in the section below

Layout/ExtraSpacing:
Enabled: true
AllowForAlignment: true
AllowBeforeTrailingComments: true
ForceEqualSignAlignment: true
Copy link
Contributor

@ColinDKelley ColinDKelley Jan 16, 2025

Choose a reason for hiding this comment

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

Do we want to force equals alignment? I think we do that about 1/3 of the time, and 2/3 we don't? I'd most like to allow it, if that's a choice.

Copy link
Author

@gfaza gfaza Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

i'll take a look at what options are available on EqualSignAlignment.

full disclosure: i am biased toward always aligning consecutive assignments, feeling that it's more readable.

at some point though, I started favoring the forcing of alignment in order to force a standard, hoping to avoid any given chunk of code from churning back and forth between alignment and non-alignment, entirely depending on the last developer's IDE auto-format settings or personal preference.

Copy link
Author

@gfaza gfaza Jan 17, 2025

Choose a reason for hiding this comment

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

if we want to allow equal sign alignment only as optional, then auto-correction could look worse than without auto-correction. this is because optional alignment allows for "over-spaced" statements while desiring to reduce only "under-spaced" statements. e.g.,

experimentally the difference between the two settings of ForceEqualSignAlignment is as follows:

a setting of ForceEqualSignAlignment: false will remove extra spacing from "under-spaced" statements (which may be worse than leaving it alone) and do nothing about "over-spaced" statements
image

whereas a setting of ForceEqualSignAlignment: true will align both "under-spaced" and "over-spaced" statements
image

this example was actually inspired by a section of code that shows why I prefer forcing a standard over enforcing no standard, and how distracting it can be when reading and/or reviewing code.

https://github.com/Invoca/web/blob/748b6c1e2a9de442b9bd8f1d408fc25581a1d5ed/app/controllers/concerns/campaigns/affiliate_campaign_summary.rb#L81-L83

image


Layout/FirstArrayElementIndentation:
EnforcedStyle: consistent

Layout/FirstHashElementIndentation:
EnforcedStyle: consistent

Layout/FirstHashElementLineBreak:
Enabled: true

Layout/HashAlignment:
Enabled: true
EnforcedHashRocketStyle: table
EnforcedColonStyle: table
EnforcedLastArgumentHashStyle: always_inspect
Copy link
Contributor

@ColinDKelley ColinDKelley Jan 16, 2025

Choose a reason for hiding this comment

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

What does this do?

Copy link
Author

@gfaza gfaza Jan 16, 2025

Choose a reason for hiding this comment

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

This enables the alignment of hash key/value pairs for hashes that use either rocket or colon-style separators.

The last directive includes the application of this styling to hashes that appear as the last argument of a method call (whether or not the hash in that position explicitly is surrounded by braces or is implicitly omitting the surrounding braces).


Layout/MultilineHashBraceLayout:
EnforcedStyle: symmetrical

Layout/MultilineHashKeyLineBreaks:
Enabled: true

# Configure cops for styles that we do not adhere to or are not agreed upon
# Default settings and options can be viewed here: https://raw.githubusercontent.com/bbatsov/rubocop/master/config/default.yml
# Please add new configurations in alphabetical order
Expand All @@ -26,12 +56,6 @@ Layout/EmptyLinesAroundClassBody:
Layout/EndOfLine:
Enabled: false

Layout/ExtraSpacing:
Enabled: false

Layout/HashAlignment:
Enabled: false

Layout/LineLength:
Max: 150
Exclude:
Expand Down
Loading