オフショア、ニアショアチームとの分散型コードレビューの進め方

オフショアチーム、ニアショアチームとのやり取りの中で、成果物レビューというのは発生します。

主に実装コードに対するコードレビューの進め方を整理したかったので、文書にまとめてみました。

個人的観測範囲で、良く観測される事象

個人的観測範囲ですが、以下のような事象を目撃しました。

  • レビューがビックバンレビューになりやすい

納品というイベント があり、そのタイミングでのレビューになりどうしてもコード規模が大きくなります。事前にすり合わせはしているとはいえ、認識齟齬や、メンバーの入れ替わりでコーディング規約が浸透していなかったりして、手戻りが大きくなりやすい傾向があります。

  • レビューが集中型レビューでレビュアーの作業負荷が高い

コードレビューで意識していること(する時/される時) - Qiita　に記載がありますが、集中型レビューでこの記事中のメリット、デメリットと同様の事象が発生しています。

  • ラボ拠点が違うと、ノウハウの共有がされない

組織、チームが異なるので難易度は高いことは間違い無いのですが、同じようなレビュー指摘を繰り返すことが多いです。

分散型コードレビュー とは？

役立つコードレビュー　8つのヒント | POSTD に、以下の記載があります。

レビューは広く分散させましょう。

一般的なよび方ではない気がしますが、複数人がレビュアーとして持ち回るケースを分散型コードレビュー と個人的に呼んでみようと思います。

何故、分散型コードレビューをしたいのか？

コードレビューで意識していること(する時/される時) - Qiita のデメリットの問題を解決したいというのが、大きい理由です。

  • 1個人に、負荷が集中することがあった。
  • ノウハウ、ナレッジが他のメンバーに貯まらない。

もう1つは1個人の負荷が下がることで、ビックバンレビュー気味になっていたレビューを時間を分けてコンスタントに実施できるようになることを期待しています。

ただ、分散型コードレビューのデメリットもあると思われ、その部分については対策をして望む必要があります。

デメリットと対策

分散型コードレビュー には、以下のようなデリメットと対策があると考えました。
分散型コードレビュー 担当者のスキルがそこまで高くないケースを想定しています。

デメリット 対策
実装の標準化の対応が難しい（指摘事項にバラツキが出る） コーディング規約の整備、コーディングルール数を多くして、その中から指摘する（定期的にルールを増やす）
レビュアーのスキルに差が出る 並行して勉強会を実施する
書いたり、見たりのコンテクストスイッチングによる集中が途切れる 実装作業を担当していないメンバーが持ち回って実施する。※ちょっとこれが効果があるかは自信はない。

前提として実施すべきもの

ニアショア、オフショアとのレビュー実施の前に、以下の点は定義しておく必要があると思いました。

  • レビュー目的をチーム内で定義する。

コードレビューを成功させるためにCTOが考えるべき7つのことー監修：高遠和也（株式会社LIG CTO） | FLEXY（フレキシー）に以下の記載があります。

「コードレビューが終了したときに、どういう状態になっていれば成功なのか」を意識したことはあるでしょうか。答えが「NO」ならば、コードレビューの実施方法そのものを見直した方が良さそうです。

分散型コードレビューだと、複数人がレビュアーになるため、より重要になると思います。

  • ニアショア、オフショアのチームと品質について合意形成する

オフショアラボと日本の品質の考え方の違いと仕組みづくり(コードレビュー編) - Sider Blog に記載されていますが、以下を実施して合意形成する必要があります。

  1. 日本のエンジニアのこだわりの「品質」を定義する
  2. オフショア側のエンジニアと品質の定義を共有する
  3. 定義を仕組み化する

日本のエンジニアのこだわりの「品質」を定義する は個人的にはコーディング規約の整備かなと考えています。

コードレビューのフェーズについて

おそらく一般的にレビューのフェーズって言わなさそうですが、ニアショア、オフショアと、社内というチームが分かれていると、開発側と、受入側でのレビューが分かれます。それをフェーズと書いています。個人的に、レビューの流れを以下のように考えています。

  • 開発側

    1. 静的解析ツール、チェックリストによる開発者自身のセルフチェック
    2. コードレビュー

  • 受入側

    1. コードレビュー、仕様整合性確認のためのコードレビュー

レビューの流れのイメージ図

レビューの流れは以下のようになるかと思います。

図にしてみました。

https://i.gyazo.com/2454fada09e0fcea7331b82dbabfa0cb.png

レビュー観点、実施方法

ニアショア、オフショア側のレビューは、チームに任せる前提で、受入側としては、以下のような観点でレビューを、するのが良さそうに思います。

Class構造

Class構造に関するレビューです。実装が書き上がる前に、Class設計ベースでレビューするイメージ。

開発規模により、実施有だったり、無かったりするかと思います。

  • 実施方法

INPUTはClass図、シーケンス図になるかと思います。

Javaであれば、AmaterasUMLを使って速攻でクラス図を書く - うめすこんぶ 等のEclipse プラグインで自動生成するのも良さそうに思います。

コード俯瞰

書き上がったコード全体を見て、構造を俯瞰するレビューです。 開発規模により、実施有だったり、無かったりするかと思います。Class構造レビューと似ていますが、コードクローン等の存在チェックや、バグの多そうな部分の当たりをつけるために実施します。

  • 実施方法

    CPDやSonarQube等のコードクローンをチェク、酷いものをリジェクトする。

    PMD、SpotBugs、SonarQube等の静的解析チェクにより、バグの偏りを確認、バグ数が多いソースを重点的に確認する。

SQL

SQLを重点的にみるレビューです。サービス仕様的に考慮する必要があるSQLの記載方法と、一般的な考慮事項があるかと思います。

  • 実施方法
    チェックリスト、コーディング規約で一般的な考慮事項を確認する。 サービスの仕様理解者が、SQLを確認して指摘する。(こっちも頻出事項は規約なり、チェックリストにして文書化する)

トランザクション

トランザクションのスタートポイント、エンドポイントが正しく実装されているかを確認するレビューです。異常系でのハンドリングが正しくできているか、等もこのレビュー観点で確認します。

  • 実施方法

シーケンス図を見ながら、トランザクションのスタート、エンドの実装を追いかけていく。

差分コード

構成管理上のDiffをベースに確認していくレビューです。細かいコーディング規約の確認、コメント妥当性

境界値などの実装妥当性を検証します。

  • 実施方法

構成管理のDiffを見ながら実施。

アドホック

観点設定をしない通常のレビューです。

いわゆるシステムの有識者、テックリード等のベテランが実施するレビューです。

非機能要件

サービス側で設定している非機能要件について確認するレビューです。

一般的に以下のような観点を確認していくのかと思います。

  • アクセシビリティ
  • パフォーマンス
  • セキュリティ・監査
  • ユーザービリティ

レビューを下読みと本レビューに分ける

レビューの規模を小さくコンスタントにしていくという部分とも重なりますが、レビュー自体も下読みと、本レビューの2回以上に分けて実施をするべきに思います。以下の記事だと、4回に分けて実施しているようです。

回数を分けるとレビュアーの集中力が戻るのと、観点を明確に分けるのはリーディング技法の記事とか、レビュー関連の書籍とか見てても効果あるって書いてあるので良さそうに思います。

参考

以下、文書作成中に見ていた記事になります。

以上です。

コメント

カテゴリー

最近の投稿