はじめに
この記事はNFLaboratories Advent Calendar 2023 - Adventar - 5日目の記事です。
お世話になってます！研究開発部の林です。本記事では、セキュリティ教育プラットフォーム開発チーム(以下、開発チーム)がどのようにプロダクトコードのレビューを実施しているか紹介します。
開発チームではスクラム体制で開発しており、レビューシステムについても日々試行錯誤と改善を重ねています。
開発チームについて
チームメンバー
まず開発チームのメンバーについての紹介です。
開発チームは、社員(専任・兼任含む)とインターン生合わせて20名弱おり、セキュリティ教育プラットフォーム開発にあたっています。
それぞれバックグラウンドも経験年数も様々で、ソフトウェア開発専門の方や、セキュリティ専門の方、自分みたいな文系学部卒の総合職でPMやったりSEやったりソフトウェア開発やったりとウロウロしている人もいます。
技術スタック
レビューに関連して、私たちが扱っている技術スタックも一部紹介します。
フロントエンド
- Javascript / Vue.js
- Tailwind
バックエンド
- Python / Django / FastAPI
- AWS
- Terraform / Ansible
- Docker
開発チームでは、デザイナーやフロントエンドエンジニアなどの区別はなく、全員がデザインからフロントエンドの実装、API/DBの設計・実装まで全レイヤー・全工程を担います。
また、GitlabのIssueベースでPBI(プロダクトバックログアイテム)を管理し、そのIssueにブランチとMR(Merge Request)を紐付けて開発を行っています。
課題
前述の通り、様々なスキルセットを持った多数のメンバーが、様々なプログラミング言語・フレームワークを使って開発をしています。さらに、開発チームではスクラムで開発しているため、自分が次に開発するIssueは基本的にはPBIの優先度上位のものから順々に取っていきます。好き嫌いや得手不得手で取り組むPBIを選ぶ訳ではありません。
加えて、常に大量のPBIが積まれており、スケジュール・納期も決して余裕がある訳ではありません。その結果、以下のような問題が出てきました。
レビューの遅延・レビュー依頼が溜まる
- 自分のタスクを優先してしまい、いつまでもレビューに取り掛かれない。
- レビューが遅延した結果、mainブランチとの差異が大きくなり、大量のコンフリクトが生じる。
レビュー時の手戻り
- マージ直前のレビュー時にDB設計の見直しなど大きな手戻りが判明する。
- コードレビューは問題なかったが成果物がPO(プロダクトオーナー)のイメージと違った。
レビュアーの偏りと知識・スキルのサイロ化
- コードベースや技術を熟知している一部のメンバーにレビュアーが偏る。
- その結果、他のメンバーが該当コード・技術に触れる機会が減り、成長が鈍化する。
レビュアー/レビュイーの心理的安全性の低下(気疲れ)
- 指摘時に実装者を傷付けないようにレビューコメントに気を遣い過ぎる。
- 全てのレビューコメントに対応しようとしていつまでもマージできない。
改善策
上述の課題に対して開発チームが実施してきた改善策の一部を紹介します。
レビューの優先度を自分のタスクよりも上げる
チームのルールとしてアサインされたコードレビューを(会議などを除いて)自分のタスク・Issueより優先することにしました。大体、レビューにアサインされた翌朝に頭の体操がてら1~2時間でレビューを返してから、自分の仕事に取り掛かるルーティンです。
ルールとして決めることで、優先度に悩んだり、レビューが放置されていつまでもマージできないという事態を防いでいます。
大きな手戻り防止のための段階的なレビュー
UIデザインも設計も基本的には個々に委ねられていますが、やはり手戻りが発生しやすいのと品質が担保できないという問題がありました。
そこで以下の方法を採用しています。
DB設計
自身が担当するIssueがデータベースへの変更を必要とする場合、詳細な実装に入る前にDB設計レビューをslackでチームに依頼し、手が空いたメンバーがレビューします。自分は下図のようにIssueのコメント上に、設計思想と共にMermaid記法でER図を書くことが多いです。メンバーからGoodマークが付くと自信を持って実装に入れます。
UIデザイン
同様に自身が担当するIssueがUI実装を含む場合、デザインがなんとなく出来た段階でPOに対してレビューを依頼して意識を擦り合わせます。一般的にはFigmaなどのプロトタイピングツールなどで共有すると思いますが、開発メンバーにとってはVueで書く方が早いので、慣例としてVueのみで行っています。レビューの仕方はスクリーンショットや画面録画をIssue上で共有したり、POと画面共有しながら動作を確認したりとやりやすい方法を採っています。
これにより、POとの認識のズレを早い段階で解消し、手戻りを防ぎます。
ランダムアサインによるレビュー機会の平準化とスキルトランスファー
Slack Botを導入してレビュアーのアサインをランダムにしています。従来の立候補型だと、常に得意な人がやったり、自分がコミットしたことあるコードのレビューを優先したり、一部の人にレビューが集中したり、などスキルと知識の偏りと不公平感が生じてしまいます。
完全にランダムにすることで、時間を経る毎に誰もが様々なレイヤーやフレームワークのコードを読めるようになります。導入初期こそ不慣れなコードを読むのに時間が掛かり効率が落ちるなどはありましたが、段々と見覚えのあるコードや実装方法が出てきて効率は上がっていきますし、他の案件でメンバーが一時的に抜けたり、長期休暇を取っても、引き継ぎに困ることがなくなりました。
スクラム的には能動的に手を挙げてIssueやレビュー依頼を取っていくべきかもしれませんが、こういったランダム要素はエンタメ性もあってメンバー間では好評です。
レビューコメントへのプレフィックス付与による温度感の可視化
googleのThe Standard of Code Review | eng-practicesを参考にしつつ、「NIT (粗探し)」や「IMO (リファクタリングやパフォーマンス向上などの提案/意見)」、「MUST (修正しないと承認できないバグ等)」などのプレフィックスをレビューコメントに付けるようにしました。
Gitlabのコメントによる非同期・非対面でのレビューのためレビュアー/レビュイーはお互いの温度感・重要度が伝わりづらいので、プレフィックスで表現するようにしています。もちろんその上で気軽に通話で質問できる空気感は大事にしています。
更なる効率化と品質向上を目指して
紹介した改善策に加え、開発チームでは更なる改善を計画しています。
Text Lint
タイポや誤字脱字、表記揺れ(サーバ or サーバー等)はどうしても起きます。教育プラットフォームの特性上、コード以外にも読み物的な学習コンテンツが多く、人力で注意するのも限界がありますし、レビューでイチイチ指摘するのもお互い骨が折れます。
とはいえ、「神は細部に宿る」とも言いますし、セキュリティを担う身としても普段から細かい点は意識したいところです。ということでText Lintで自動で直していこうという試みです。前述の通り、扱う単語がとにかく多いので、徐々に辞書を増やしていき、効率化を図る予定です。
AI (ChatGPT/Cursor)
弊社ではすでにChatGPTが使えますが、巷ではGithub Copilot ChatやCursorなど、エディタ上でファイルやリポジトリを参照しながらアドバイスしてくれる対話型AIツールが話題です。我々は研究もミッションとしているため、これらのAIツールも積極的に導入・検証をしていき、実質AIと常時ペアプログラミングができるような体制を目指しています。
社内勉強会での水平展開
これは自分が現在企画中です。日々たくさんのIssueが消化されていく中、自分としては実装時に創意工夫したけどちゃんと共有する間もなくslackの彼方に流されていってしまった、とか、タメになるレビューコメントを貰ったからみんなにも共有したい、とか、そんな悩みや気づきを共有する場を設けようと思います。
スクラムイベントとしてのレトロスペクティヴ(スプリント毎の振り返り)では、スクラムについての振り返りが主になりますが、やはり技術の振り返りや共有も大事かなと思います。これもまた別のブログで取り組み結果を報告できればと思います。
最後に
自分が4月に弊社にジョインしてからの8ヶ月弱での、開発チームのコードレビューにおける課題と改善策と今後の取り組みを紹介しました。引き続きチーム全員で改善していきますのでまた共有させていただきます！
また、自分のチームではコードレビューでこんな工夫をしてるよとか失敗談とかあれば是非ともお聞かせください！
