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

  2. 自己紹介 Akihiko Sato / 株式会社ラクス Lead Engineer / @akkiee76 SaaS

    開発 (Backend, Frontend) / Mobile 開発 (iOS, Android) 上流工程、コードレビュー、チームの課題改善など 読書 / コーヒー / HHKB / 体幹トレーニング
  3. コードレビュー技術について ・オブジェクト指向で類型化するコードレビュー ・レビューガイドラインで技術力を見える化する ・コードレビューの心構え

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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

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