From 9d566a2250130285efe30ca59e3248678c1b7ac3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 19:43:56 +0200 Subject: [PATCH 1/8] Remove most performance rubocop rules 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. --- .rubocop.yml | 51 --------------------------------------------------- 1 file changed, 51 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index d2733f69c..19ff3f619 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -12,21 +12,9 @@ Gemspec/OrderedDependencies: Gemspec/RequiredRubyVersion: Enabled: true -Performance/Caller: - Enabled: true - -Performance/CaseWhenSplat: - Enabled: true - -Performance/Casecmp: - Enabled: true - Performance/CompareWithBlock: Enabled: true -Performance/Count: - Enabled: true - Performance/Detect: Enabled: true @@ -36,48 +24,9 @@ Performance/DoubleStartEndWith: Performance/EndWith: Enabled: true -Performance/FixedSize: - Enabled: true - -Performance/FlatMap: - Enabled: true - -Performance/RangeInclude: - Enabled: true - -Performance/RedundantBlockCall: - Enabled: true - -Performance/RedundantMatch: - Enabled: true - -Performance/RedundantMerge: - Enabled: true - -Performance/RegexpMatch: - Enabled: true - -Performance/ReverseEach: - Enabled: true - -Performance/Size: - Enabled: true - Performance/StartWith: Enabled: true -Performance/StringReplacement: - Enabled: true - -Performance/TimesMap: - Enabled: true - -Performance/UnfreezeString: - Enabled: true - -Performance/UriDefaultParser: - Enabled: true - Rails/ActiveSupportAliases: Enabled: true From ea17256e3a7137cd6057c6dd087dbe8d48c30c8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 19:46:42 +0200 Subject: [PATCH 2/8] Remove gemspec rubocop rules We don't have a gemspec file, so we don't need these ones. --- .rubocop.yml | 9 --------- 1 file changed, 9 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 19ff3f619..795de3e58 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -3,15 +3,6 @@ inherit_from: .rubocop_basic.yml Bundler/InsecureProtocolSource: Enabled: true -Gemspec/DuplicatedAssignment: - Enabled: true - -Gemspec/OrderedDependencies: - Enabled: true - -Gemspec/RequiredRubyVersion: - Enabled: true - Performance/CompareWithBlock: Enabled: true From 9fb1d7df3125468bea03b4f0e2c5d2dd5ff093fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 22 Jun 2019 19:51:41 +0200 Subject: [PATCH 3/8] Remove obsolete ScopeArgs rubocop rule This rule doesn't make sense with Rails 5 anymore, since breaking it will raise an error. --- .rubocop.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 795de3e58..66763e9ea 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -90,9 +90,6 @@ Rails/ReversibleMigration: Rails/SaveBang: Enabled: true -Rails/ScopeArgs: - Enabled: true - Rails/SkipsModelValidations: Enabled: true From d639cd587a33a327b1d127815aa288fac8f49aa0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 00:28:02 +0200 Subject: [PATCH 4/8] Remove unnecessary `uniq` calls 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. --- .rubocop.yml | 3 --- app/controllers/concerns/moderate_actions.rb | 2 +- .../tracking/budget_investments_controller.rb | 2 +- lib/user_segments.rb | 12 ++++++------ 4 files changed, 8 insertions(+), 11 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 66763e9ea..1e882cfd8 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -93,9 +93,6 @@ Rails/SaveBang: Rails/SkipsModelValidations: Enabled: true -Rails/UniqBeforePluck: - Enabled: true - RSpec/AlignLeftLetBrace: Enabled: false diff --git a/app/controllers/concerns/moderate_actions.rb b/app/controllers/concerns/moderate_actions.rb index f8c2b96c8..52e0a02cf 100644 --- a/app/controllers/concerns/moderate_actions.rb +++ b/app/controllers/concerns/moderate_actions.rb @@ -25,7 +25,7 @@ module ModerateActions @resources.accessible_by(current_ability, :ignore_flag).each(&:ignore_flag) elsif params[:block_authors].present? - author_ids = @resources.pluck(author_id).uniq + author_ids = @resources.pluck(author_id) User.where(id: author_ids).accessible_by(current_ability, :block).each { |user| block_user user } end diff --git a/app/controllers/tracking/budget_investments_controller.rb b/app/controllers/tracking/budget_investments_controller.rb index 4586b7308..83a4f28d0 100644 --- a/app/controllers/tracking/budget_investments_controller.rb +++ b/app/controllers/tracking/budget_investments_controller.rb @@ -48,7 +48,7 @@ class Tracking::BudgetInvestmentsController < Tracking::BaseController def heading_filters investments = @budget.investments.by_tracker(current_user.tracker&.id).distinct - investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id).uniq) + investment_headings = Budget::Heading.where(id: investments.pluck(:heading_id)) .order(name: :asc) all_headings_filter = [ { diff --git a/lib/user_segments.rb b/lib/user_segments.rb index 17d609e68..d6bd427fb 100644 --- a/lib/user_segments.rb +++ b/lib/user_segments.rb @@ -18,29 +18,29 @@ class UserSegments end def self.all_proposal_authors - author_ids(Proposal.pluck(:author_id).uniq) + author_ids(Proposal.pluck(:author_id)) end def self.proposal_authors - author_ids(Proposal.not_archived.not_retired.pluck(:author_id).uniq) + author_ids(Proposal.not_archived.not_retired.pluck(:author_id)) end def self.investment_authors - author_ids(current_budget_investments.pluck(:author_id).uniq) + author_ids(current_budget_investments.pluck(:author_id)) end def self.feasible_and_undecided_investment_authors unfeasible_and_finished_condition = "feasibility = 'unfeasible' and valuation_finished = true" investments = current_budget_investments.where.not(unfeasible_and_finished_condition) - author_ids(investments.pluck(:author_id).uniq) + author_ids(investments.pluck(:author_id)) end def self.selected_investment_authors - author_ids(current_budget_investments.selected.pluck(:author_id).uniq) + author_ids(current_budget_investments.selected.pluck(:author_id)) end def self.winner_investment_authors - author_ids(current_budget_investments.winners.pluck(:author_id).uniq) + author_ids(current_budget_investments.winners.pluck(:author_id)) end def self.not_supported_on_current_budget From 3fa3801a49feb6b2f8df95d9bc34e37af0e2e37f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 00:51:58 +0200 Subject: [PATCH 5/8] Remove unused rails rubocop rules We consider these rules either return false positives or we don't have a strong opinion about them. --- .rubocop.yml | 39 --------------------------------------- 1 file changed, 39 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 1e882cfd8..157363d47 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -18,24 +18,9 @@ Performance/EndWith: Performance/StartWith: Enabled: true -Rails/ActiveSupportAliases: - Enabled: true - -Rails/Blank: - Enabled: true - Rails/CreateTableWithTimestamps: Enabled: true -Rails/Delegate: - Enabled: true - -Rails/DelegateAllowBlank: - Enabled: true - -Rails/DynamicFindBy: - Enabled: true - Rails/EnumUniqueness: Enabled: true @@ -45,9 +30,6 @@ Rails/EnvironmentComparison: Rails/Exit: Enabled: true -Rails/FilePath: - Enabled: true - Rails/FindBy: Enabled: true @@ -63,27 +45,12 @@ Rails/HasManyOrHasOneDependent: Rails/InverseOf: Enabled: true -Rails/LexicallyScopedActionFilter: - Enabled: true - Rails/NotNullColumn: Enabled: true -Rails/Output: - Enabled: true - Rails/OutputSafety: Enabled: true -Rails/Present: - Enabled: true - -Rails/ReadWriteAttribute: - Enabled: true - -Rails/RedundantReceiverInWithOptions: - Enabled: true - Rails/ReversibleMigration: Enabled: true @@ -183,12 +150,6 @@ RSpec/NestedGroups: Enabled: true Max: 4 -RSpec/PredicateMatcher: - Enabled: true - -Rails/HttpStatus: - Enabled: true - RSpec/RepeatedDescription: Enabled: true From 16b8bee1405cad4c411b79fa0f0c455558617a98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 02:21:43 +0200 Subject: [PATCH 6/8] Remove unused rspec rubocop rules We consider these rules either return false positives or we don't have a strong opinion about them. --- .rubocop.yml | 105 --------------------------------------------------- 1 file changed, 105 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 157363d47..606a449b2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -60,114 +60,9 @@ Rails/SaveBang: Rails/SkipsModelValidations: Enabled: true -RSpec/AlignLeftLetBrace: - Enabled: false - -RSpec/AlignRightLetBrace: - Enabled: false - -RSpec/AnyInstance: - Enabled: false - -RSpec/BeEql: - Enabled: true - -Capybara/FeatureMethods: - Enabled: false - -RSpec/ContextWording: - Enabled: false - -RSpec/DescribeClass: - Enabled: true - -RSpec/DescribeMethod: - Enabled: true - -RSpec/DescribeSymbol: - Enabled: true - -RSpec/ExampleLength: - Enabled: false - -RSpec/ExpectActual: - Enabled: true - -RSpec/ExpectInHook: - Enabled: true - -RSpec/ExpectOutput: - Enabled: true - -RSpec/FilePath: - Enabled: true - -RSpec/ImplicitExpect: - Enabled: true - EnforcedStyle: should - -RSpec/InstanceSpy: - Enabled: true - RSpec/InstanceVariable: Enabled: true -RSpec/InvalidPredicateMatcher: - Enabled: true - -RSpec/ItBehavesLike: - Enabled: true - -RSpec/IteratedExpectation: - Enabled: true - -RSpec/LeadingSubject: - Enabled: true - -RSpec/MessageChain: - Enabled: true - -RSpec/MessageExpectation: - Enabled: true - -RSpec/MessageSpies: - Enabled: true - EnforcedStyle: receive - -RSpec/MultipleDescribes: - Enabled: true - -RSpec/MultipleExpectations: - Enabled: false - -RSpec/MultipleSubjects: - Enabled: true - -RSpec/NamedSubject: - Enabled: false - -RSpec/NestedGroups: - Enabled: true - Max: 4 - -RSpec/RepeatedDescription: - Enabled: true - -RSpec/ReturnFromStub: - Enabled: true - -RSpec/SharedContext: - Enabled: true - -RSpec/SingleArgumentMessageChain: - Enabled: true - -RSpec/SubjectStub: - Enabled: true - -RSpec/VerifiedDoubles: - Enabled: true - Security/Eval: Enabled: true From 7e9e1be1aa2f094ae85becc880a0edfce10f2023 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sun, 23 Jun 2019 02:43:43 +0200 Subject: [PATCH 7/8] Remove Bundler/InsecureProtocolSource rubocop rule Since we're using rubygems as the only source, this rule is not necessary. --- .rubocop.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index 606a449b2..093bdcb00 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,8 +1,5 @@ inherit_from: .rubocop_basic.yml -Bundler/InsecureProtocolSource: - Enabled: true - Performance/CompareWithBlock: Enabled: true From 97ec8776abf8d215ee0b5ad516c396f1ef1fe1a9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Tue, 10 Sep 2019 22:04:53 +0200 Subject: [PATCH 8/8] Reduce severity of LineLength rule 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. --- .rubocop_basic.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.rubocop_basic.yml b/.rubocop_basic.yml index 88fdf0f9c..041ccd935 100644 --- a/.rubocop_basic.yml +++ b/.rubocop_basic.yml @@ -122,6 +122,7 @@ Lint/UselessAssignment: Metrics/LineLength: Max: 110 + Severity: refactor Rails/ActionFilter: Enabled: true