自分がどう動作確認しているかを書きましょう

コードだけでなく、動作確認方法もレビューしてもらいましょう

動作確認方法がレビューできれば、手元で動かさなくてもLGTMできる、かもしれない

こちらに詳しい話があるので読みましょう

/shokai/Pull Requestに動作確認方法を書け

例

コメント

このPRでは、cssのファイルの配置を変えているだけなので、比較的簡単なPRである。

ここで、「project設定の見た目が崩れていない」という項目があることに注目。つまり、本人がここが変わる可能性があると認識して、確認したことを示している。（実際 sassのimport順が変わっているのでその恐れは若干ある）

また、レビューアーも、この点を再度確認すると共に、さらに漏れていないところがないか考える。

的には、scrapboxのshokaiの「動作確認」の項目が非常にレビューしやすくて毎回すごいと思っている

が、案外自分で書いてみると難しいということに気づいている。

Railsのほうのレビューをするとき、同じことを書いて欲しいと思うので、みんなマネして欲しい。

GyazoRailsのPullReq templateに How to check it という項目があって、同じことを意図しているのですが、それとは別の話？

もっと細かく書くべし、という話なのか、書いてない人がいるので書いてください、という話なのか

shokaiさんのブログには同意

rakusaiさんがRailsの方のレビューで同じことを書いて欲しいと思う時があれば、その都度このページにリンクを貼りつつ指摘して行くと良さそうですね

ホントに説明が書かれていなかったら書けよで済むんだけど、説明が不十分とかだと気づけないので、都度都度指摘してちゃんと書くような文化をつくるべきだと思うので

How to check it とも大まかな趣旨は同じだと思います。

PRは非常に読みやすいし、ミスが少なくてよいのでお手本にしましょう。という話。

↑に例をあげてみました。

how to check it の書き方のコツと言い換えてもよいです。

言いたかったこと

自分の所に来るOSSのpull requsetにレビュー方法とか一切書いて無くてmergeできない

もうコードが頭に入ってないので、その変更の影響範囲を想像できない

じゃあpull request送る側の頭にある影響範囲を教えてくれよというのがきっかけ

例えばさっき作ったこのPullReq、How to check itの欄で課金ページに影響があることが分かるけど、それとは別に影響範囲も明示した方が分かりやすいのかな

adblockインストールしていない時は表示されない事を確認してますよね？それも書くといいと思う

すごく自明っぽいけど影響範囲を漏れなくカバーする具体的な確認フローを書いておくといい気がしてる

お、なるほど

実装した部分だけ書きがちなので参考になった

レビュアーと変更者が互いに、「あのへんまで影響してそうだけど◯◯氏ならまあチェックしてるだろ、たぶん」とか考え出すとつらい

もう一度書き直すと更に良い

pull requestのレビューというのは、結局ダブルチェックなのだから

まずチェック項目に漏れが無い事を確認するべき

ある人が「この操作で全てを確認できてる」と判断した「判断そのもの」を、別の人が確認する

設定画面に項目追加したけど、これログイン画面にも影響しているよね、とか

（必要があれば）チェック項目を2人とも実行してみて、本当にokか確認する

実は同じ確認操作を二人でやっても、ほぼ意味がない

コンピュータでは、2人が同じ操作やったら基本的には同じ結果しか得られない

各人のローカルのデータセットが違う等、色々な前提条件が違う環境でtestしてみると見つかるbugはある。そういう場合だけやればいい

悪いレビュー依頼

確認項目を明かさずレビューを依頼すると

両者がランダムに確認するだけになる

両者が「相手なら自分の想定外の影響範囲もチェックしてくれてるだろ・・たぶん・・」という適当なLGTMを出す

LGTM（見た感じ良いんじゃね？）

どこを見たのかが重要

バグを探す時に、どこを見ていないかで推理できる

PullRequestの手引き に追加したい