diff options
author | Lin Jen-Shin <godfat@godfat.org> | 2019-04-01 18:55:46 +0300 |
---|---|---|
committer | Lin Jen-Shin <godfat@godfat.org> | 2019-04-01 18:55:46 +0300 |
commit | 700e8d1917462f509e430f78f7effca8df56bdba (patch) | |
tree | e1589388079b1a7f6b3302171edbc9c767730abc | |
parent | f87ff7af538a6ac559217a4c7cfa0fd75f247fd6 (diff) | |
parent | 98a5976b787fad0797bc5e3231c48ab3f400bce6 (diff) |
Merge branch 'enable-rubocop-application-record' into 'master'
Document ApplicationRecord / pluck_primary_key
Closes #59690
See merge request gitlab-org/gitlab-ce!26764
-rw-r--r-- | .rubocop.yml | 14 | ||||
-rw-r--r-- | doc/development/sql.md | 15 | ||||
-rw-r--r-- | lib/tasks/tokens.rake | 2 | ||||
-rwxr-xr-x | scripts/insert-rspec-profiling-data | 2 |
4 files changed, 31 insertions, 2 deletions
diff --git a/.rubocop.yml b/.rubocop.yml index 9143966b864..648d59e8062 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -146,6 +146,20 @@ Naming/FileName: - XSS - GRPC +Rails/ApplicationRecord: + Enabled: true + Exclude: + # Models in database migrations should not subclass from ApplicationRecord + # as they need to be as decoupled from application code as possible + - db/**/*.rb + - lib/gitlab/background_migration/**/*.rb + - lib/gitlab/database/**/*.rb + - spec/**/*.rb + - ee/db/**/*.rb + - ee/lib/gitlab/background_migration/**/*.rb + - ee/lib/ee/gitlab/background_migration/**/*.rb + - ee/spec/**/*.rb + # GitLab ################################################################### Gitlab/ModuleWithInstanceVariables: diff --git a/doc/development/sql.md b/doc/development/sql.md index 47519d39e74..edeca7fb298 100644 --- a/doc/development/sql.md +++ b/doc/development/sql.md @@ -155,6 +155,21 @@ The _only_ time you should use `pluck` is when you actually need to operate on the values in Ruby itself (e.g. write them to a file). In almost all other cases you should ask yourself "Can I not just use a sub-query?". +In line with our `CodeReuse/ActiveRecord` cop, you should only use forms like +`pluck(:id)` or `pluck(:user_id)` within model code. In the former case, you can +use the `ApplicationRecord`-provided `.pluck_primary_key` helper method instead. +In the latter, you should add a small helper method to the relevant model. + +## Inherit from ApplicationRecord + +Most models in the GitLab codebase should inherit from `ApplicationRecord`, +rather than from `ActiveRecord::Base`. This allows helper methods to be easily +added. + +An exception to this rule exists for models created in database migrations. As +these should be isolated from application code, they should continue to subclass +from `ActiveRecord::Base`. + ## Use UNIONs UNIONs aren't very commonly used in most Rails applications but they're very diff --git a/lib/tasks/tokens.rake b/lib/tasks/tokens.rake index eec024f9bbb..46635cd7c8f 100644 --- a/lib/tasks/tokens.rake +++ b/lib/tasks/tokens.rake @@ -24,7 +24,7 @@ namespace :tokens do end end -class TmpUser < ActiveRecord::Base +class TmpUser < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord include TokenAuthenticatable self.table_name = 'users' diff --git a/scripts/insert-rspec-profiling-data b/scripts/insert-rspec-profiling-data index 10e337b9972..b34379764e0 100755 --- a/scripts/insert-rspec-profiling-data +++ b/scripts/insert-rspec-profiling-data @@ -22,7 +22,7 @@ module RspecProfiling ENV['RSPEC_PROFILING_POSTGRES_URL'] end - class Result < ActiveRecord::Base + class Result < ActiveRecord::Base # rubocop:disable Rails/ApplicationRecord acts_as_copy_target end end |