🐷
負債だらけの5年ものコードに、初めてRuboCopを導入するけど、面倒な未来が見えたので現実的にこうした
前置き
状況としてはタイトル通りです。
苦しみが滲むタイトルになってしまいましたが、ヤンチャなスタートアップだとあるあるなのではないでしょうか。
方針を議論したポイント
- 過去分は auto fixしちゃう?
→ さすがに怖すぎるので、周辺を実装した時についでに修正していくスタイルにする = 修正したfileだけ違反チェック対象とする - 過去の違反分を全てignoreをすると、ignoreをする癖がついてしまうのでは?
→ RuboCop todoは導入しない - とはいえ過去の規約違反のせいで、スピードが落ちることは避けたい
→ 特にpatchが必要な時とか死ぬので、修正しないとマージできないという事態は避けたい
→ 過去分で落ちまくりなので、commitする毎に警告が出るのもストレスになる = pre-commitに入れない - 一行の文字数制限など、鬱陶しくなりがちなものは入れない
方針の結論
- RuboCop todoは使わない
- チェック対象は修正したfileのみ
- CIのstepに入れる
- けれども失敗してもCIは成功させ、修正は任意にする
- チェック項目はBug検知に役立ちそうなものにする
CIはこんな感じ(イメージ)
git diffでdevelopブランチとの差分だけrubocopのチェックをかけている
そして失敗したとしてもechoで直してね!とエラー部分とメッセージが出るだけで、CI事態はpassする
# rubocop
- when:
condition:
# envをenumで管理している。dev envの時にだけ実行。
equal: [ "dev", "<< parameters.env >>" ]
steps:
- run:
name: Run rubocop
command: |
bundle exec rubocop $(git diff --name-only --diff-filter=d origin/develop | grep -e '\.rb$') || echo "better to fix"
.rubocop.ymlはこんな感じ
最低限bug検知に役立ちそうなものを、公式や他社事例を見ながら選出
require:
- rubocop-performance
- rubocop-rails
- rubocop-rspec
AllCops:
TargetRubyVersion: x.x.x
TargetRailsVersion: x.x.x
NewCops: enable
# defaultで全てdisableにしておいて、欲しいやつだけ明示的にenableにする
DisabledByDefault: true
# 除外するディレクトリ(自動生成されたファイル)
# デフォルト設定にある"vendor/**/*"が無効化されないように記述
Exclude:
- "vendor/**/*" # rubocop config/default.yml
- "db/**/*"
- "config/**/*"
- "bin/*"
- "Gemfile"
# Rails向けのRails copsを実行
Rails:
enabled: true
Performance:
Enabled: true
Security:
Enabled: true
Bundler:
Enabled: true
Bundler/OrderedGems:
Enabled: false
# disableコメント内のcopの部署名の補完
Migration:
Enabled: true
# 潜在bug検知に役立ちそうなもの
Lint/AmbiguousAssignment:
Enabled: true
Lint/BooleanSymbol:
Enabled: true
Lint/CircularArgumentReference:
Enabled: true
Lint/DuplicateCaseCondition:
Enabled: true
Lint/DuplicateElsifCondition:
Enabled: true
Lint/DuplicateHashKey:
Enabled: true
Lint/DuplicateMethods:
Enabled: true
Lint/DuplicateRegexpCharacterClassElement:
Enabled: true
Lint/DuplicateRequire:
Enabled: true
Lint/DuplicateRescueException:
Enabled: true
Lint/EmptyExpression:
Enabled: true
Lint/EmptyInterpolation:
Enabled: true
Lint/FormatParameterMismatch:
Enabled: true
Lint/FloatOutOfRange:
Enabled: true
Lint/SafeNavigationChain:
Enabled: true
Lint/IneffectiveAccessModifier:
Enabled: true
Lint/MissingCopEnableDirective:
Enabled: true
Lint/NestedMethodDefinition:
Enabled: true
Lint/NumberedParameterAssignment:
Enabled: true
Lint/OrAssignmentToConstant:
Enabled: true
Lint/PercentSymbolArray:
Enabled: true
Lint/StructNewOverride:
Enabled: true
Lint/TripleQuotes:
Enabled: true
Lint/UselessElseWithoutRescue:
Enabled: true
Lint/UselessMethodDefinition:
Enabled: true
Lint/UselessSetterCall:
Enabled: true
Lint/Void:
Enabled: true
所管
負債が溜まってくると根本解決は容易ではありません。
それでもできることをやっていくことが、何より心理的に大事だと思っています
Discussion