Upgrade to Pro — share decks privately, control downloads, hide ads and more …

こんなコードレビューは嫌だ

akkie76
October 11, 2022

 こんなコードレビューは嫌だ

akkie76

October 11, 2022
Tweet

More Decks by akkie76

Other Decks in Technology

Transcript

  1. 〜 生産性を高めるコードレビューのTIPS 〜
    こんなコードレビューは嫌だ
    @akkiee76

    View full-size slide

  2. 自己紹介
    Akihiko Sato / 株式会社ラクス Lead Engineer / @akkiee76
    SaaS 開発 (Backend, Frontend) / Mobile 開発 (iOS, Android)
    上流工程、コードレビュー、チームの課題改善など
    読書 / コーヒー / HHKB / 体幹トレーニング

    View full-size slide

  3. コードレビュー技術について
    ・オブジェクト指向で類型化するコードレビュー
    ・レビューガイドラインで技術力を見える化する
    ・コードレビューの心構え

    View full-size slide

  4. 今日伝えたいこと
    「嫌なPR/MR」を避けることで
    コードレビューの生産性と品質を高めよう!

    View full-size slide

  5. こんなコードレビューは嫌だ ①
    changes / diff が多すぎる

    View full-size slide

  6. レビューで困ること
    ・考慮漏れが発生するリスクが比例する(品質への懸念)
    ・レビューに時間がかかる
    ・手戻りのリスク(修正時のコストが大きい)

    View full-size slide

  7. 原因考察
    ・実装スコープが大きい
    ・不要なファイルがコミットされている(node_moduleなど)

    View full-size slide

  8. アクションプラン
    ・実装スコープを細かく分けて、changes を少なくする
    ・一部の条件分岐のロジックのスコープを分ける
    ・横展開系開発は、基本方針と横展開を分ける
    ・ライブラリのアップデートは機能ごとに分ける

    View full-size slide

  9. こんなコードレビューは嫌だ ②
    overview の情報が少ない
    実装方針が分からない

    View full-size slide

  10. レビューで困ること
    ・レビューに時間がかかる
    ・実装者との意思疎通・コード解釈
    ・手戻りのリスク(実装方針の認識相違)

    View full-size slide

  11. 原因考察
    ・レビュワーへの配慮
    ・設計の認識合わせ不足

    View full-size slide

  12. アクションプラン
    ・overview に実装方針を記載する
    ・必要に応じて実装の背景をコメントで残す
    ・「FYI 」などのコメント略語を利用する

    View full-size slide

  13. こんなコードレビューは嫌だ ③
    コミットコメントと内容が一致しない

    View full-size slide

  14. こんなコードレビューは嫌だ ③(実例)
    ・実装5
    ・実装4
    ・実装3
    ・実装2
    ・実装1
    commit の粒度が
    分からない・・・

    View full-size slide

  15. レビューで困ること
    ・全部の chages / diff でレビューを行うことになる
    ・レビューに時間がかかる
    ・考慮漏れのリスク(場合によって)

    View full-size slide

  16. 原因考察
    ・実装プロセスが明確になっていない(惰性で実装)
    ・設計の認識合わせ不足

    View full-size slide

  17. アクションプラン
    ・commitコメントと内容をなるべく一致させる
    ・実装プロセスと内容を確立させる(頑張り過ぎない)

    View full-size slide

  18. こんなコードレビューは嫌だ ④
    リファクタリングが含まれている

    View full-size slide

  19. レビューで困ること
    ・問題が発生した場合、問題箇所を特定しにくい(品質)
    ・軽微な修正は許容
    ・chages / diff が多くなりがち
    ・レビューに時間がかかる

    View full-size slide

  20. 原因考察
    ・実装プロセスが明確になっていない
    ・影響範囲の調査不足

    View full-size slide

  21. アクションプラン
    ・影響範囲は事前に調査する
    ・リファクタリングはスコープを分ける
    ・可能であれば事前にリファクタリングできると GOOD !

    View full-size slide

  22. こんなコードレビューは嫌だ ⑤
    ライブラリのアップデートが含まれている

    View full-size slide

  23. レビューで困ること
    ・問題が発生した場合、問題箇所を特定しにくい(品質)
    ・依存関係により影響範囲が大きくなるケースがある

    View full-size slide

  24. 原因考察
    ・影響範囲が少なそうなので、ついでにアップデートした
    ・IDEで良しなにアップデートされていた
    ・diff を確認で漏れてしまった

    View full-size slide

  25. アクションプラン
    ・ライブラリのアップデートのみをスコープにする
    ・影響範囲に関わらず機能チェックするとよい
    ・バージョン開発早期にやると GOOD !

    View full-size slide

  26. こんなコードレビューは嫌だ5選
    ・changes / diff が多すぎる
    ・overview の情報が少ない
    ・コミットコメントと内容が一致しない
    ・リファクタリングが含まれている
    ・ライブラリのアップデートが含まれている

    View full-size slide

  27. まとめ
    開発を進める上で大切なこと
    ・PRのスコープを大きくしない
    ・実装者とレビュアーのコミュニケーションを大切にする
    ・diff を確認する
    明日からのコードレビューでぜひ実践してみよう!

    View full-size slide

  28. ご清聴ありがとうございました

    View full-size slide