ここ数年たくさんレビューする機会に恵まれ、より良いレビューとは何かを考えていたのでまとめてみた
レビューをなぜ行うか
大きく分けて以下3点
- コードの品質向上
- 属人性の排除
- レビュイー/レビュアーの技術力の向上
コードの品質向上
- 要件を満たしているか
- バグが存在しないか
- コーディング規約に沿っているか
→Linter等でチェックすべきであるが、現実問題整備されてないことがほとんど
- テストコードはあるか、内容は適切か
- 可読性、保守性に問題はないか
- パフォーマンス問題を引き起こさないか
→DBアクセス部分等を特に注視。将来的なデータ量も予測する
このあたりは必ずレビューする必要があり、基本的には機械的なチェックが行えず人力でチェックする必要がある
チェック項目の中でも以下のように分けることが出来て、必要な技量がそれぞれ異なる
- PJの知識、業務知識
- 要件を満たしているか
- パフォーマンス問題を引き起こさないか
- テストコードはあるか、内容は適切か
- 技術に対する知識
- バグが存在しないか
- コーディング規約に沿っているか
- テストコードはあるか、内容は適切か
- 可読性、保守性に問題はないか
- パフォーマンス問題を引き起こさないか
技術力が高い=適切なレビュー指摘が出来るとはならないのが難しいところで、技術と業務知識が合わさって初めて適切なレビューが行えると考えている
だからと言って業務知識がまだ少ないからレビューは避けよう…というのは誤りで、むしろ知識が少ないうちにたくさんレビューは行ったほうが良い
業務知識があるメンバー同士だと暗黙知によるレビュー漏れや、潜在的な問題を後回しにするといった問題が起きる可能性もあるので
業務知識がない新規メンバーから見たフレッシュな観点でレビューするのがとても大切(レビュー指摘ではなく質問だけでも良い)
早いうちからレビューを行うことで以下のようなメリットが有る
- コードベースでやり取りが出来るので質疑応答がスムーズ
- PJの全体的なコードを知るキッカケになり、開発が進めやすくなる
- 積極的な姿勢を見せることで信頼が生まれ、開発に意見を出しやすくなる
- 新人がレビューすることでレビューに対する敷居を下げることが出来る
レビュー経験自体が無く手も足も出ない場合は以下のような手法をとっても良い
- ペアレビューという形で有識者/経験者と一緒にレビューを行う
- 理解できている部分に対して、自分の認識があっているかという質問を投げる
- COOLなコードを褒める
キッカケさえあればその後は段々レビューが出来るようになるはずなので、まずは簡単なところから始めるのが良いはず
素敵なコードに対して:cool:とか:+1:とかつけるだけでも十分価値があるので、どんどんつけていくと良いと思う
→:cool:貰えた側も嬉しいし良い事尽くし。出来る人のコードって段々褒められなくなるし褒めていきたい
属人性の排除
どんな開発手法でも大体は一人のメンバーに対してタスクがアサインされるので、その時点で属人性が高まってしまう
大きめのタスクであれば更に細分化し複数人で分担する事で属人性が減らせるが、それでも担当箇所はどうしても属人性が高まる
属人性が高いと急な事故や病気で担当者が作業できなくなった場合、引き継ぎが難しくなるという大きなリスクを抱えることになる
※無理に属人化を避けようとすると、個人がコードを書かないように自動生成ですべて行えるようにして…等、逆に開発効率が落ちるようなケースもあるので、必ずしも属人化が悪いというわけではない
属人性の排除において特に有効なのがコードレビューだと思う
実装内容自体はVCSで管理していたらコミット履歴やPRから把握することは出来るが、実装時の思想や意図を確認するのはレビューのタイミングしかない
レビューで思想や意図を把握しておくことで、今後その箇所を触るときに元の設計を壊さないようにしたり、もし問題があれば修正を行うことも容易になる
把握していないと修正して良いのかわからず、怖いから一旦そのままに…でどんどん技術的負債が溜まっていく事が多々ある
レビュイー/レビュアーの技術力の向上
まず、これは人それぞれな部分ではあるが、目的もなくただ技術の勉強をするより目的を持って技術の勉強を行ったほうが効率よく学習が行えると考えている
(技術書読むだけで全部わかる人は考慮しない)
レビューは技術力の向上においてはベストな場所で、効率の良い学習が行える以下の条件が揃っている
- 解決すべき問題がある
- 多人数で議論が行える
- たたき台となる答えがまず提示されている
単に勉強しろと言われてもまず何からやるべきか…と詰まってしまうが、レビューは上記条件があるのでまず詰まることはない(レビューの仕方がわからない等は除く)
マージするために必ず答えが導き出されるので終わりが無いということもない
レビュアーは質問を行ったりコードを読むことで技術力が向上し、レビュイーは質問に回答することで技術力の向上がする
特に問題なければ質問自体が生まれないが、コードを読むだけでも技術力が向上するので問題ない
レビューを行うべき人
色々考えを書いたが、ではレビューは誰が行うべきなのか?
個人的にはチームメンバー全員がすべきだと思っている
大体のプロジェクトだとレビューを行う人/行わない人は以下だと思う
- 行う人
- 実際にコードを書いている開発者同士
- レビュー担当者
- 行わない人
- プロジェクト管理者(PL, PMなど)
- QA/テスター
当然コードを読める人がレビューしたほうが早いし、より適切なレビューを出来るので間違っていないと思う
しかし開発者だけがレビューすると、開発独自の目線でしか見ることが出来ず、コードのみの品質が向上しソフトウェアの品質は横ばい(最悪の場合低下)する可能性があると考えている
行わない人それぞれがレビューした場合に生まれるメリット
プロジェクト管理者
大体はタスク管理、進捗管理を行いユーザとのやり取りを行うのみ
仮にレビューを行った場合以下のメリットが考えられる
- 各タスクの進捗が把握しやすい
- 障害発生時の開発者とのやり取りがスムーズになる
- 開発中の仕様変更に対して、現状の実装をベースにユーザと話が出来る
- 仕様の誤りに気づき手戻りを防ぐことが出来る
開発を滞りなく進め、ソフトウェアの完成が成果となる管理者ではつい進捗のみに目を向けがちだが
レビューを行うことでより詳細な進捗を把握できることになり、適切な納期提案やユーザとの相談が行える様になると思う
一部のエンジニアは「コードを書けない人は信用しない」と言う考えを持つエンジニアもいるため、その層に対しても
「書けない(時間がない)が、レビューで貢献して成長しようとしている」という姿を見せることで、リーダーの役割に対して理解を得られる可能性もあると思う
QA/テスター
大体は実装自体は行わず、品質の向上のためにテストケースの作成やテストの実施するのみ
仮にレビューを行った場合以下のメリットが考えられる
- 実装内容を考慮したテストケースの作成やテストが行える
- バグ発見時の開発者とのやり取りがスムーズになる
- 軽微なバグであれば自分で修正を行い、開発者の手を止めることが無く対応が行える(開発者はレビューだけでOK)
責務的にレビューは行わないことがほとんどだと思うが、レビューすることでソフトウェア自体の品質向上に大きく効果があると思う
特にやり取りをスムーズに行える点がかなり有効で、おそらくこのあたりでしょうか?とコードベースで話すことでかなり修正が行いやすくなるはず
開発者としても実装仕様を理解しているテスターだとやりやすいので、開発とテスター間で信頼が生まれると思う
まとめ
レビュー自体はコードを一行も生み出さないので開発スピードが低下するイメージもあるが、バグを防いだり技術力の向上が見込めたりで実装する以上のメリットがあるのでどんどん行うべきだと思う
みんな大好きt_wadaさんもテストコードを書かないと最初は開発スピード上がるけど、時間が経てば経つほど修正が難しくなって開発スピードが落ちていく、という話をしていたので似たような物だと考えている
レビュー自体が無いPJでも自分からレビューの基盤を整えてレビュー環境を作っていくのは誰でも出来るので、積極的にアプローチすると:cool:
🐊個人的な話🐊
レビューめっちゃ好きなんすけど、経験則でずっとレビューしてきたので体系的にレビューを学べる本とかあれば教えて下さい
考え方とかレビューの勘所とか何でも知りたいです。押忍。