よい Pull Request は修正点が見つけやすい
本記事は、メディキューの社内資料を公開用に編集したものです。主に、メディキューでソフトウェアエンジニアリングを始めるインターンや医療従事者の方々を対象としています。
はじめに
プログラミングを始めたばかりで、Git や GitHub を使うのが初めて、という人にとっては Pull Request のテキストに何を書いていいかわからない、ということも多いでしょう。よい Pull Request はメディキューに積み上げる資産になるし、自分の仕事を円滑に進めてくれるのでよい書き方を学ぶことが大切です。Pull Request のことをよく知らなかったり、これからも触る可能性が低い職種の方は「Pull Request = 社内提案資料 = 稟議」のようにざっくり理解すると良いでしょう。
この記事では下のような人に向けています。
- Pull Request を出すための Git や GitHub の操作方法はわかる
- でもどのような Pull Request の内容を目指すべきかわからない
修正すべき点が見つけやすいとレビュアーに優しい
そもそもなぜいきなり main branch に修正を入れずに Pull Request を介するかというと、「レビューを行い修正点の発見を行うため」です。実際には、知識の共有とかログの参照性とかもありますが今回はこの点のみにフォーカスします。
上の目的を受け入れて、まず良いレビューを定義するとすれば「修正するべき点があったときにそれがすべて見つけた」レビューが良いレビューということになります。とすると、当然ながら修正するべき点があったときにそれをレビュアーが発見しやすい Pull Request がよい Pull Request ということになります。
Pull Request 作者の責務 - 「確かにそれしかない」と思わせる
Pull Request に description がなんのために存在するのかは様々な考えがあると思います。筆者の考えでは「自分たちが達成したいことに近づくにはこのコード変更が最適である」という主張をするためにあります。レビュアーを「確かにそれしかないな」と一瞬で納得させて Approve させる Pull Request が最高です。これを行うためには自分がなぜその変更を行うべきだと考えているのか、他の選択肢はないのか、なぜこの変更が間違っていないのか、副作用はないのか、など様々な情報を整理する必要があります。この情報の整理こそ Pull Request 作者の責務です。
Pull Request レビュアーの視点 - より良い可能性を見つける
レビュアーを納得させるには、レビュアーの視点の理解も重要です。「納得させる」というような表現をしたので少し対立するような立場にも見えますが、前提としてレビュアーは「一緒に最適なコード変更を行っていく仲間」であり、あくまで正しいチームの意思決定のために「問題を探す担当」です。レビュアー目線での良い Pull Request でのやりとりは「作成者が気付けなかったより良い変更の可能性を提示してより良い形に促す」ことです。ここでレビュアーからよい指摘を引き出すには、レビュアーが気になる論点を先に潰しておく事、自分の結論に誤りがあったら気づきやすい情報を示すことが重要です。
反証可能性と再現性
後述しますが修正点の発見という目的には下のことが重要です。
- 反証可能性
- 再現性
科学に似ています。これは下に示す工学の定義(の一つ)が根底にあります。
実際的な問題に対する効率的/経済的な解を見つけるための経験的、科学的なアプローチの応用
『継続的デリバリーのソフトウェア工学』 より
我々はソフトウェアエンジニアリングをしていて、そのエンジニアリングに科学的アプローチの応用が入っているからです。
WHY と WHAT を書く
要件だけから良いものを作るのは難しいのでフォーマットの理解から入りましょう。メディキューでは WHY と WHAT を書くことをフォーマットにしています。それぞれ何を達成したいのか、そのために何をするのかを書くものです。
WHY と WHAT は入れ子構造
どこからが WHY でどこからが WHAT かを厳密に議論し出来なくて構いません。なぜなら WHY と WHAT は入れ子構造になるからです。
- 例: データの力で医療の価値を最大化する
- → データ品質を維持する技術をもつ
- → データ品質が下がったときに知ることができる様にする
- → データ品質を正しく定義する
どれも親から見たら WHAT だけど子供から見たら WHY になります。本来すべての会社におけるアクションは何らかの形で(どれだけ遠回りになっても)会社のミッションにつながるはずです。とするとなぜこのコード変更をするのか?という疑問は突き詰めていくとメディキューでは「データの力で医療の価値を最大化する」に行き着くはずです。このすべての関係を整理するのは大変なのでメディキューでは Business / Product / Project / Epic / Task という階層を用意しているわけです。
せめて1段階くらいは整理されている必要がある
Pull Request を書いてみてなにが WHY で何が WHAT かよくわからないという経験は多くの人があるかと思います。上の入れ子構造があるのである意味それはそのはずです。毎回そのすべてを整理するのはコスパが悪いので、その WHY のチェーンの1段階くらいは整理しておこう、というのが WHY と WHAT を書く理由です。
今あなたがそのコード変更を行うべきだと思ったのには何らかの理由があるからです。その理由とやったことの関係性を整理してください。入ったばかりのインターンだと「こういう指示を受けたから」ということの理解がなかなかできていないこともあると思いますが、「X というファイルの N 行目に Y という文字列を入れろ」というレベルの指示ではもらっていないはずで「何らかの達成したいゴール」としての WHY を提示されているはずです。その達成したいゴールが WHY でそのゴールに辿り着くまでにやったことが WHAT になります。
WHY- なぜその Pull Request が必要なのか?
WHY を Issue で先に十分に整理するのか Pull Request で整理するのかは大きな問題ではないので状況に合わせて読み替えてください。Issue と Pull Request の使い分けもまた WHY と WHAT の使い分けくらい話が長くなるので Issue に慣れていない人は Pull Request の中に書く WHY の話だと思って問題ありません。
さて、冒頭にで話した「修正するべき点」の話を思い出しながら WHY の書き方を考えていきましょう。「修正するべき点」が見つけやすく、別の言い方をすると 反証を提示しやすい Pull Request が良い Pull Request です。その達成のためには反証を用意しやすいように「自分がこのコード変更が最適だ」と考えるロジックを整理する必要があります。そのロジックの出発点になる WHY が間違っているとその後のロジックの全てに意味がなくなってしまいます。WHYから整理することでセルフレビューとして「WHY を書いてみたら他の方法もありそうに思えてくる」とか「この WHY は本当は存在しない課題に見える」とかいったことが起こります。これ自体が Pull Request の品質を上げてくれるし、レビュアーもこのレベルから話を疑わなくて良くなるので楽です。
反証を用意しやすい情報を記載
まずケーススタディから入りましょう。
「バグがあるのを直したい」
よく見る説明で確かに「バグは直すべきだろう」と思ってしまうのでスルーされがちですが、この WHY のチェーンを辿っていくときになぜ会社のミッション実現につながるのか結構謎です。
下のような気になるポイントがあります
- 極論今後絶対に使わないコードならバグなら直す必要はない
- むしろ消すべき
- それは本当にバグなのか
- そのバグが実質的に仕様として受け入れられてはいないのか
- 極端な例として 「
Google
はGoogol
のタイポ」が事実だとしてもはや直すべきではない
上のような疑問が生じないようにするためには
「XXという期待に反してYYが起こるという事象が実際に生じてユーザーに不利益がでているのでこれを解消したい」の様に書けばいいでしょう。これがあると下のような反論が可能になります。
- その不利益は実質ゼロなので直すコストの方が大きい
- もしくは「そのYYは起こってない」とか「ユーザーはYYを期待している」とか
- そもそもその機能を消すほうがユーザーに利益が大きい
こういう情報は反証できる WHY を書こうとしたら自分で気付けることも多いのでしっかり書くことは重要です。実際には直すべきであることが自明なバグは「XXとなるべきところYYになっているのを直したい」くらいで簡略化して書いてしまうこともあります。ただし簡略化するときでも「真面目に整理するとこんな感じなんだろうな」というイメージをいつでも持てるようにしておきましょう。
実務上は Issue の URL を記載するだけで多くの場合困らない
そもそも Issue を整理するタイミングでその変更が本当に必要かは議論されている事が多いでしょう。したがってその Issue リンクを示すだけで「この Issue が紐付く Epic を達成するため」という情報が十分読み取れます。ただし読みやすさのためにその Issue の概要を要約したりタイトルをコピペしてあげるとレビュアーに優しいでしょう。また、Issue に WHY が適切にかけていることも前提になる結局このロジックの整理はどこかでやる必要があります。基本的には避けて通れません。
Tips
- 代替手段の集合の共通点を考える
- なんとなく理解している WHY の言語化におすすめ
- 例: 「お茶をコンビニで買ってくる」というタスクだと認識したら
- 「水をスーパーで買ってくる」でも良さそう、と考える
- 「自動販売機でジュースを買ってくる」もありかもしれない、と考える
- でも「酒を通販で買うはダメそう」ともわかる
- -> 今回は「短時間で冷えたノンアルコールドリンクを提供したい」が WHY なのかも、と考える
- -> 更にふかぼるともしかしたら「今日のイベント参加者の熱中症リスクを下げたい」かもしれない。だとするともっと良い手段も提示できるかもしれない
- 絶対に今考えている手段でないといけない理由、を考える
- 「代替手段集合の外」を考えるイメージ
- そうすると「代替手段集合」の形が良く見えてくる
- 「なぜやることになったか?」ではない
- 経緯に反証は提示できない
WHAT - WHATを達成するのになぜそのコード変更が適切なのか?
WHAT では「今回取った手段がwHYの解決に最適である」ことを主張してください。何をしたか、は極論コードを読めばわかるので説明が必要だと思ったときだけで問題ありません。
考えると良い観点は「選択肢の正しさ」と「変更の正しさ」の2点があります。
選択肢の正しさ
そもそもなにかを実装するときは選択肢を複数出してその中から適切なものを選ぶという考えが必要です。これについては簡単に下の記事の 1.a にまとめています。
すべてのあり得る選択肢を並べられたか、そのすべての中から最適なものなのかを整理することでレビュアーのより良い変更の可能性を提示する能力を引き出しやすくなります。選択肢が一つしか思いつかなかったり、これしかないという自明に思えることもあるでしょう。その場合は「他に思いつかなかった」と正直にかいてしまっても問題ありません。それがあるだけでもレビュアーは「他の選択肢を考えてみると『最適である』という主張に反証が提示できそう」と考えることができます。
変更の正しさ
数ある選択肢の中から適切なものを選べていたとしても実際のコード変更が正しいとは限りません。問題は解決していないかもしれないし逆効果が何処かで生まれているかもしれません。そこで「自分の変更で正しい」という主張をするためのエビデンスを添えましょう。このとき再現可能性が重要になります。
問題が解決している証明
まずそもそも達成したかった目標が達成されていることを示しましょう。メディキューで多い例は SQL の修正だと思いますが、「修正後の SQL を実行してみると、不整合のある行が0件になった」といった主張をすることが多いでしょう。これをレビュアーが再現しやすいようにSQLの全文を書いてあげると良いでしょう。
逆効果がない証明
目標が達成できていても他の問題を同時に産んでしまっては意味がありません。逆効果がないエビデンスも添えましょう。例えば下のようなことがあるでしょう。
- 例: あるコードやファイルが消えたときに
- そのコードがなくていい理由
- もう参照されていない
- 絶対に条件分岐の中に入らない
- その責務が他のコードに引き継がれたことを示す
- そのコードがなくていい理由
- 例: あるコードを別のコードに置き換えたとき
- そのコードの機能に差分がない理由、(もしくはより多くなっている)
Tips
- そもそも「正しさを担保する実験」を再現しやすくするといい
- リンクを追加するのはそれ
- 検証コードを全文貼る
- 自動化に頼ろう
- 「このテストが通っているから」と言えるとように普段からテストを書いておく
- 「型チェックが通っているから」と言えるように型は丁寧に書いておく
- メディキューでは正しくない変更はできる限りCIが落ちるように固めにテストがあるが「なぜこのテストで自分の変更が正しいと言えるか」を示すのは作者の責務
例
このセクションはメディキューのメンバー向けです。リンクはすべてアクセス権限のあるメンバーに限られます。
-
https://github.com/medicu-inc/one/pull/2965#issuecomment-2439821589
- WHY / WHAT の description を添削して書き直した例
-
https://github.com/medicu-inc/one/pull/3027
- レビュアーの視点からどんな事が書いてあるとよりレビューしやすいかを説明した例
終わりに
頑張りすぎなくて ok
色々と細かいことを書いてきましたが、すべてをガチガチに守る必要はありません。あくまでレビュアー、他のチームメンバー、AI、将来のチームメンバーなどに向けて適切に情報が伝われば問題ありません。細かく情報を整理するほうが時間がかかってコスパが悪いことも多いでしょう。そこであまり肩肘張らずに少しずつどういった情報を提示すれば仕事がスムーズに進んでいくのかを掴んでいけば ok です。
いつでも本気を出せる自信は必要
ただし「精緻に整理したロジックを記載する」ということをやろうと思えばいつでもできるという自信も大事です。できるうえで「これくらい簡略化しても全然伝わるし間違わない」とスキップするのとわからないまま進むのでは積み上げていったときの仕事のクオリティに大きく差が出ます。そういう意味では最初の方に頑張って細かく整理する訓練を積むのもいいでしょう。
コスパのいい方法を見つけていこう
最終的には適切な Pull Request を書くという「実際的な問題」に対して「効率的/経済的な解」を見つけていくとどんどん楽になっていきます。例えばリンクを書くだけで伝わるものはリンクを書いておくとか、何回も検証するような項目は自動化しておくとかをしておくとすべてを細かくやらなくても適切に「問題点を見つけてもらう」という目的が達成されていくようになります。ソフトウェアエンジニアをするうえで記録やコミュニケーションについてもエンジニアリングしていきましょう。
Discussion