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

Remove unused rubocop rules #3637

Merged
merged 8 commits into from
Sep 10, 2019
Merged

Remove unused rubocop rules #3637

merged 8 commits into from
Sep 10, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented Jun 24, 2019

References

Background

There are many rules in our .rubocop.yml file which were defined years ago. Since then, we've added HoundCI to automatically warn us about coding style issues.

We think many of them are either too strict or they report false positives, which might be confusing for contributors.

Objectives

  • Remove rules we think could be too overwhelming

@javierm javierm changed the base branch from change_basic_rubocop_rules to add_spacing_rules June 24, 2019 13:30
@javierm javierm changed the base branch from add_spacing_rules to change_basic_rubocop_rules June 24, 2019 13:30
@javierm javierm force-pushed the change_basic_rubocop_rules branch 2 times, most recently from be81033 to 60949bb Compare June 24, 2019 13:39
@javierm javierm self-assigned this Jun 24, 2019
@javierm javierm force-pushed the change_basic_rubocop_rules branch from 60949bb to 4a68574 Compare July 5, 2019 00:21
@javierm javierm force-pushed the change_basic_rubocop_rules branch from 4a68574 to 654b974 Compare July 5, 2019 00:30
@javierm javierm force-pushed the change_basic_rubocop_rules branch from 654b974 to d519471 Compare July 5, 2019 01:27
@javierm javierm force-pushed the change_basic_rubocop_rules branch from d519471 to b6fd326 Compare July 5, 2019 01:30
@javierm javierm force-pushed the change_basic_rubocop_rules branch from b6fd326 to b11c5aa Compare July 5, 2019 01:38
@javierm javierm force-pushed the change_basic_rubocop_rules branch from b11c5aa to f72301a Compare July 5, 2019 03:25
@javierm javierm force-pushed the change_basic_rubocop_rules branch from f72301a to 2406de8 Compare July 5, 2019 03:44
@javierm javierm force-pushed the change_basic_rubocop_rules branch from 2406de8 to 39ba010 Compare July 5, 2019 12:57
@javierm javierm added this to Testing in Roadmap Sep 10, 2019
@javierm javierm moved this from Testing to Doing in Roadmap Sep 10, 2019
@javierm javierm force-pushed the change_basic_rubocop_rules branch 2 times, most recently from c04ea2c to 2243809 Compare September 10, 2019 19:43
@javierm javierm changed the base branch from change_basic_rubocop_rules to master September 10, 2019 19:53
@javierm javierm moved this from Doing to Testing in Roadmap Sep 10, 2019
@javierm javierm moved this from Testing to Reviewing in Roadmap Sep 10, 2019
For performance purposes, we need to find bottlenecks in our
application. Optimizing the performance of small methods doesn't make
the application faster.

I've kept a few cops because applying these ones IMHO make the code
easier to read.
We don't have a gemspec file, so we don't need these ones.
This rule doesn't make sense with Rails 5 anymore, since breaking it
will raise an error.
The code `where(id: ids)` is equivalent to `where(id: ids.uniq)`.

Since Rails 5 uses `distinct` instead of `uniq` and in most cases where
we use `uniq` with `pluck` we should simply remove the `uniq` call (as
done in this commit), we're also removing the `Rails/UniqBeforePluck`
rubocop rule.
We consider these rules either return false positives or we don't have a
strong opinion about them.
We consider these rules either return false positives or we don't have a
strong opinion about them.
Since we're using rubygems as the only source, this rule is not
necessary.
Since we're ignoring this rule in many places, we're marking it in a
different way so it's clear we're not as strict with this rule.
@javierm javierm moved this from Reviewing to Testing in Roadmap Sep 10, 2019
@javierm javierm merged commit 4b77557 into master Sep 10, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Sep 10, 2019
@javierm javierm deleted the remove_rubocop_rules branch September 10, 2019 21:05
@javierm javierm added Refactoring Linters Rubocop, ERB Lint, ESLint, SCSS-Lint, ... and removed Refactoring labels Sep 11, 2019
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Linters Rubocop, ERB Lint, ESLint, SCSS-Lint, ...
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

1 participant