🐷

負債だらけの5年ものコードに、初めてRuboCopを導入するけど、面倒な未来が見えたので現実的にこうした

2023/03/30に公開

前置き

状況としてはタイトル通りです。
苦しみが滲むタイトルになってしまいましたが、ヤンチャなスタートアップだとあるあるなのではないでしょうか。

方針を議論したポイント

  • 過去分は 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