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.

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

Update to latest rubocop gems; simplify rubocop.yml #58

Merged
merged 1 commit into from
Jul 15, 2020
Merged

Update to latest rubocop gems; simplify rubocop.yml #58

Show file tree
Hide file tree
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
34 changes: 4 additions & 30 deletions .rubocop.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ require:
AllCops:
DisplayCopNames: true
DisplayStyleGuide: true
NewCops: enable
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This automatically opts us into new rules that are added in future versions of rubocop. So we no longer need to explicitly list the new cops with Enabled: true every time we update the gems.

Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

TargetRubyVersion: 2.4
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought that this was also unnecessary... that it would default to whatever ruby version is run.

See rubocop/rubocop#5610.

Should we let the version autodetect instead? It's one fewer places to change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My understanding is that TargetRubyVersion can be inferred for app repos, because an app is deployed with a specific version of ruby (usually declared in the Gemfile). But for library code like socrates, we support multiple versions and so that inference can't be done. So we need to be explicit about the lowest version of ruby that this library supports.

If I remove the TargetRubyVersion line, rubocop defaults to 2.7. It is not smart enough to infer based on the required_ruby_version in the gemspec.

Exclude:
- "bin/*"
Expand All @@ -21,37 +22,22 @@ AllCops:
Layout/CaseIndentation:
Enabled: false

Layout/EmptyLinesAroundAttributeAccessor:
Enabled: true

Layout/FirstArrayElementIndentation:
EnforcedStyle: consistent

Layout/HashAlignment:
Enabled: false

Layout/LineLength:
Max: 120
Comment on lines -33 to -34
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is now the default for rubocop so we don't have to explicitly specify it.


Layout/MultilineMethodCallIndentation:
EnforcedStyle: indented

Layout/SpaceAroundMethodCallOperator:
Enabled: true

Lint/AmbiguousBlockAssociation:
Enabled: false

Lint/RaiseException:
Enabled: true

Lint/ScriptPermission:
Exclude:
- "Rakefile"

Lint/StructNewOverride:
Enabled: true
Comment on lines -52 to -53
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because of NewCops: enable, we no longer need to explicitly opt in to all these rules.


Metrics/AbcSize:
Max: 35
Exclude:
Expand Down Expand Up @@ -84,6 +70,9 @@ Performance/Casecmp:
Security/YAMLLoad:
Enabled: false

Style/AccessorGrouping:
Enabled: false
Comment on lines +73 to +74
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This one caused violations in this repo so I turned it off. Enabling it would mean we would lose our ability to add comments for each individual accessor:

attr_accessor :one # Comment describing :one
attr_accessor :two # Comment describing :two

Rubocop wanted:

attr_accessor :one, :two


Style/BarePercentLiterals:
EnforcedStyle: percent_q

Expand All @@ -96,21 +85,9 @@ Style/Documentation:
Style/EmptyMethod:
EnforcedStyle: expanded

Style/ExponentialNotation:
Enabled: true

Style/FrozenStringLiteralComment:
EnforcedStyle: never

Style/HashEachMethods:
Enabled: true

Style/HashTransformKeys:
Enabled: true

Style/HashTransformValues:
Enabled: true

Style/Lambda:
EnforcedStyle: literal

Expand All @@ -131,6 +108,3 @@ Style/StringLiterals:

Style/StringLiteralsInInterpolation:
EnforcedStyle: double_quotes

Style/StructInheritance:
Enabled: true
4 changes: 2 additions & 2 deletions socrates.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ Gem::Specification.new do |spec|
spec.add_development_dependency "bundler", "~> 2.1.2"
spec.add_development_dependency "rake", "~> 13.0"
spec.add_development_dependency "rspec", "~> 3.8"
spec.add_development_dependency "rubocop", "= 0.83.0"
spec.add_development_dependency "rubocop-performance", "= 1.5.2"
spec.add_development_dependency "rubocop", "= 0.88.0"
spec.add_development_dependency "rubocop-performance", "= 1.7.0"
spec.add_development_dependency "simplecov", "~> 0.16"
spec.add_development_dependency "timecop", "~> 0.9"

Expand Down