意識のズレを感じたら As-Is / To-Be がおすすめ ― チームでコードレビューに対する意識調査をした話

wakamsha
0

As-Is / To-Be とは?

現状と理想のギャップを的確に認識し、取り組むべき課題を抽出したうえで行動に移すためのビジネスフレームワークです。

As-Is:
現在の状態がどうであるかを表すもの。
To-Be:
理想の状態を表すもの。実現可能かどうかの制約を受けない純粋な理想(= あるべき姿)のことを指す。

主に計画や戦略の立案時に利用されるものですが、チームビルディングにおいても有効です。
理想のチームに到達するには解決すべき課題が多数存在するものですが、そういった課題が明確に把握されてることは稀です。解決するには対象である課題を明確に把握せねばなりません。

As-Is / To-Be モデルは、現状と理想とのギャップを洗い出すだけのツールです。しかし、そこから「何が欠けているのか」「何をすべきなのか」を客観的に可視化できる非常に有益なフレームワークなのです。

コードレビューが難しいのはメンバー間の意識にズレがあるから

チーム一丸となってソフトウェア開発をするうえでコードレビューはなくてはならないものです。お互いのコードを読みあうことで新たな知識を会得したり改善案を提案しあう(= 切磋琢磨)のはもちろん、コミュニケーションを密に取ることでチームの結束力を高めるなど、実に多くの効果をもたらしてくれます。

しかしそんな大切なコードレビューであっても、人によって意識の度合いはまちまちです。どれくらいの優先度で臨むべきなのか、どこまで詳細にレビューすべきなのか、どこまでレビューイの判断に委ねるべきなのか等など…。この瞬間はコードという成果物に対して直接干渉するため、コミュニケーションもセンシティブになりがちです。弊チーム内にも(直接言葉にはせずとも)コードレビューに対するモヤモヤ感がどことなく漂っていました。

そこでチーム内で As-IS / To-Be を使った意識調査を行い皆で振り返る場が設けられました。ここからはケーススタディとしてその内容をご紹介します。

【As-Is】コードレビューの優先度はどれくらいに置いてますか?

プルリクエスト(以下、プルリク)は予めスケジューリングされるものではなく突然やってきます。レビュワーもまた自身の開発タスクやミーティングなどの業務を抱えているため、そこからレビューに当てるリソースを捻出することになります。

以下はそんなコードレビューの優先度に対する現状の回答です。

itsukichang itsukichang

  1. Slack 上でのテキストコミュニケーション
  2. QA 対応など緊急性の高いタスク
  3. 会議など自分以外の関係者がいるタスク
  4. コードレビュー 👈
  5. 自分の開発タスク
  6. その他雑務など

リマインダーからの通知時(毎日 11:00 と 17:00)にプルリクの中身に目を通し、小規模ですぐ済ませられそうなら即レビューする。
会議中のスキマを縫ってレビューすることも。

他タスクに追われてるときは、業務終了間際か翌営業日の朝イチにまとめて取り掛かるようにしてる。
そういうときにリマインダー通知が役に立つ(翌朝に持ち越して万が一忘れても思い出せる)。

リマインダーとは、 web フロントエンド関連リポジトリでオープン中のプルリクを Slack のチャンネルへ定期的に通知される仕組みのことです。弊チームでは、毎日 11:00 と 17:00 の二回通知が来るように設定しています。

レビュワーがアサインされていれば Slack の通知も同時に発火するので便利。

kazuma1989 kazuma1989

  1. 日直
  2. 打ち合わせ準備
  3. 事務
  4. コードレビュー 👈
  5. 自分の開発タスク

11:00 の通知が来たタイミングで見始めることが多い。
GitHub Projects のカンバンに挙がってるタスクをチェックする中で目についたら着手することも。

amyu amyu

  1. (予めスケジューリングされている)会議
  2. Slack 上でのテキストコミュニケーション
  3. QA 対応など緊急性の高いタスク
  4. コードレビュー 👈
  5. 自分の開発タスク

Slack のチームチャンネルに毎日通知されるリマインダーのうち 17:00 の回を見てレビュワーになっているものをチェックしている。

hey3

プルリクオープンから2,3時間以内に見れたらいいなという意識でいる。
リマインダーからの通知のうち、自分にアサインされているプルリクはブラウザタブで開いておき、時間を見つけて着手するようにしている。

定期的に awaiting review from you に目を通す。

それ以外にも Slack に流れてくる GitHub 通知の中から目についたものは(レビュワーでなくとも)積極的に見に行くようにしている。

fukumasuya fukumasuya

  1. (予めスケジューリングされている)会議や緊急の障害対応
  2. その日のうちに捌く必要のある急ぎのタスク
  3. コードレビュー 👈
  4. 自分の開発タスク
  5. その他雑務など

自分としては優先度高めに置いている(つもり)。
とはいえ、1, 2 に追われて翌日に持ち越したり、そのまま忘れて数日放置してしまうしまうことも...…。

チーム向けリマインダーとは別に、13:00 と 15:00 に Slack の 自分向け DM で Github から通知を受け取るようにしている。

自分向けの DM として個別に通知を受け取るようにしているとのこと。

wakamsha wakamsha

優先度は高め。頻繁にチェックするようにしている。
もともと集中力が長続きしないので、コーディングの最中でも手を止めてレビューし始めることもしばしば。

なるべくレビュワーがブロッカーにならないよう意識している(同様に自分が出したプルリクエストも放置せず早めに見てほしいという想いもある)。

他タスクに追われてすぐに見れないときは、とりあえず 👀 リアクションを付けることで「忘れてないよ」という意思表示をしておく

プルリクの規模が大きいときは、スキマ時間でコメントをこまめに差し込んだりしてる。
リマインダー通知だけでなく GitHub トップページにある Recent activity 一覧を見て気付くことも多い。

GitHub トップページに自身が関係するプルリクや Issue のダイジェストが表示される。

あくまで当人がどのように考えているかについての自己申告なので、ここに客観性はありません1)そもそも誰かを吊るし上げるのが目的の場でもありません。。こうして実際に吐露することで自身の思考を整理するだけでも効果としては充分と言えます。

【To-Be】どれくらいの優先度でやるのが望ましいか?

自分の開発タスクより優先度が高ければ良いかと。

As-Is でメンバー全員が「自分の開発タスクよりは優先する」と回答したとおり、ここは全員が一致しました。

以下はその他に挙げられた回答を抜粋したものです。

レビューが滞って後続タスクのブロッカーにならないかは意識したが良さそう。
滞っていたら(= 放置されていたら)Slack で直接催促していい。
プルリク作成者側も自身の後続タスクに待ちが発生しないよう上手く切り分けるなど工夫する余地はある。

Hot Fix など急ぎの案件なら、プルリクオープンと同時に Slack で周知すればいい。

「急ぎではない」というのをイレギュラーケースとして表明すると良いのでは。
レビューは優先度高いタスクであることを基本原則にしておく方が、チームとしては良いと思う。
実装 -> レビュー -> マージ -> 実装 ... というサイクルをたくさん回していくのは、チームにとって良いことのはず。

どうしてもレビューする時間が取れないときもある。
そういうときは、カジュアルに他の人にバトンタッチしていいルールがあると良さそう。

弊チームのリポジトリは GitHub の機能でレビュワーを自動的にアサインするよう設定されていますが、必ずしもこれを遵守する必要はなく、メンバー各人が妥当と判断すれば他メンバーにアサインし直しても構わないということです。コードの保守は実装者やレビュワー個人ではなくあくまでチーム全体の責務であるため、メンバーの判断はチームの判断というわけです。

そしてここに挙げられたものは、 既に実行できているものばかりでなく、新しい取り組みとしての改善案も含まれています。つまり「本当はもっとこうした方が良いのではないか」と以前より抱えていたものもあれば「自分はそこまで課題に感じてなかったけど、どうやら他メンバーはそうでもなかったらしい。ならばこういう取り組みはどうだろうか。」と外からの刺激で新しいアイデアが出てくることもあるわけです。

【As-Is】レビュー時はどのようなところに注目しているか?

コードレビューはソフトウェアの品質に大きく影響する重要な作業ですが、基本的にレビュワーは目視でコードを読み解くため、一字一句全てに目を通して完璧に解読するのは難しいこともあります。

以下は、メンバーが普段どういった観点でレビューに望んでいるのかについての回答です。

コーディングに関する観点

変数や関数のネーミングは適切かどうか。
関心の境界に不自然なところはないかどうか。
必要に応じて JS Doc コメントは書かれているかどうか。

ドメイン知識がそこまで無い人でも無理なく意味を汲み取れるかどうかを重視している。

コードの記法(≒ フォーマット等)に関しては「ESLint に怒られていなければ大丈夫なのだろう」と見なしているので、あまり気にしない。

新機能追加であれば、コンポーネントや(状態管理)ストアの責務に自分にとっての「驚き」がないかどうかを意識して見ている。
そういった既存コードから大きく逸脱したものがなければ、自分が代替案を思い付くことはあっても、実装者の判断を(代替案を取り入れるかどうかも含めて)尊重するようにしている。

DRY にこだわり過ぎていないかどうか。
その時は美しく共通化できたとしても度が過ぎると後の機能追加等に脆くなってしまうため、無理してる雰囲気がないかを入念に見るようにしている。

※ 極端に DRY に徹したコードは大量コピペのコードよりも保守性に劣ることがある。

TODO コメントは適切に書かれているかどうか。
時間が経って当時のことを忘れてしまっても思い出せるように。
引継ぎ時にも役に立つ。

プルリク説明やプルリクコメントに留めてしまうにはもったいないと思ったコメントは、コードコメントにコンバートしてもらうようにしている。

弊チームには、コードそのものの保守と同じくらいドキュメントの保守にも注力する風習があるため、コードレビューでは TODO コメントや JS Doc コメントが適切に書き残してるかを注力する傾向にあるようです。

ドメイン知識に関する観点

全体的にドメイン知識がそこまでなくても理解できるようなコードになっているかどうか。
暗黙知に頼りすぎた実装であればあるほど属人的なコードになってしまい、後から新しく参画するメンバーがキャッチアップに苦労してしまう。
古参メンバーでも時間が経てば忘れてしまう。

基本的に実装者を信頼しているので、仕様を満たしているかどうかの深堀りはしない。
仕様まで追いかけはじめると、Jira チケットや Confluence などにも目を通すことになり、コードレビューの域を超えてしまうので。

どのみち本番環境リリースまでに QA や PdM チェックが入るので、余程のことが無い限り大丈夫だろうと見なしている。

運用に関する観点

プルリクのメタ情報が適切に設定されているかどうか。

CI/CD の結果はオールグリーンになっているかどうか。
Visual Regression Test の差分は妥当かどうか。

QA が必要か不要かについての指示文。
必要ならその分のリソースとスケジュールは調整済みかどうかまでチェックする。

その他

入社してまだ日が浅い身なので、キャッチアップを兼ねて弊チームのコーディング規約や既存コードと照らし合わせつつ全体像を見るようにしている。

自分や他メンバーが過去のレビューで指摘を受けたところと同じような記述はないかを見るようにしている。

  • e.g.
    • CSS の書き方
    • カスタムフックの切り出し方
    • ファイル構成やコンポーネントの粒度
    • etc...

CSS に苦手意識があるので、あえて積極的に見るようにしている。
これはレビューと言うよりも「なにか新しい知見がないかな」といった程度のもの。

自身が苦手なところを積極的に読みに行って学ぶのは、技術者としてとても大切な姿勢だと思い知らされました。まさに目からウロコです。

この質問に関しては現状そこまでチームとしての課題を感じていなかったらしく To-Be までは行われませんでしたが、As-Is で各メンバーの思考が可視化されただけでも大きな収穫と言えます。

KPT との違い

KPT(Keep Problem Try) もまたチーム内の課題を抽出して改善のための行動に移すためのツールであり、As-Is / To-Be と通ずる部分があります2)弊チームもおよそ月に一回の頻度で実施しています。。最終的にチームの改善を促すためのツールという点では両者は似ていますが、課題抽出に対するアプローチに違いがあります。

KPT ではその名の通り Problem といういま現在チームに潜んでいる課題を抽出するフェーズが明確にあり、そこでは「このチームに〇〇という課題がある」の様なわりかし直接的な表現で問題提起されがちです。すべてはチームをより良くするためであり、そのために忌憚のない意見を表明して Try (改善策)をハッキリと打ち出すことは大切なのですが、表現を誤ると人によっては耳の痛い内容として受け止められる場合もあります。指摘内容が全て正しければまだしも、そこに情報の非対称性(≒ 提起者による誤解)が存在すると軌道修正をする(≒ 誤解を解く)手間が発生してしまいます。

As-Is / To-Be で抽出するのはあくまで現状と理想のみであり、今ある課題と改善策を明確に打ち出す機能まではありません。そこはチームの自主性に委ねることができるため、KPT よりもカジュアルな性質を備えてると言えます。最終的に改善策を打ち出してチーム一丸となって行動できればどちらでも構いませんが、ケースバイケースで使い分けられるよう両者の特性を知っておくと良いかもしれません。

締め

いかがでしたか?

これはあくまで僕個人の感想ですが、今回これを実施する前と後でチームのコードレビューに対する振る舞いに変化が見られました。「うちのチームはこういうルールにしよう」とハッキリ決めたわけではないのですが、To-be で挙がったことがそれとなく実践されてコードレビューがより活発になった印象です。

As-Is / To-Be を使うと各メンバーが気にしているポイントが言葉として共有でき、その上で自身以外の意識や気にしているところが参考になることがあります。

もしチーム内で意識のズレを感じたときに、チームとしてどういう方針を掲げるのが良いのかについて建設的な議論に持ち込めたり、その場で運用改善までできてしまうこともあります。議論の結果、最終的に無理してまで足並みを揃えないという結論に至っても良いのです。他メンバーがどう考えているのかを知れるだけでも大違いです。

手軽に始められるので、ぜひとも一度お試しあれ。

脚注

脚注
1 そもそも誰かを吊るし上げるのが目的の場でもありません。
2 弊チームもおよそ月に一回の頻度で実施しています。