コードレビュー
@imos さんのつぶやき 8:30 PM - 1 Oct 2016
コードレビュー，皆が自分のタスクよりも優先的にやってくれるので時間かかる感覚はないし，間違いを見つけられるだけではなく，知識（バグを減らす書き方とか，C++11ではこう書くのが良いとか，社内の新しいスタイルとか）を共有して議論でき学べる場でもあるのでとても良いですよ
［翻訳］コードレビューについて | iwashi.co
TL; DR
1. コードレビューはおそらく、常に最優先であるべきだ。 あなた自身のイベントループにコードレビューを組み込むための最適な方法を、理解しておくこと
2. レビュー依頼をするときは、内容をレビュワーが読んだ時に嬉しいようにすること
個人的には、チームとして生産性をあげるためにもっとも重要なのは、全てのチームメンバがコードレビューを最優先として意識することだ。私の思いつく、もっとも説得力がある簡潔な理由は以下のとおりなので、強調して述べる：
コードレビューを先延ばしにするのは、スレッド処理をブロックしていることと同義だ
レビューが溜まっていたり、依頼された PR の近辺をあまり把握できていなくて、時間がかかりそうな場合は正直に 「◯◯ で時間がかかりそうなので、急ぎであれば他の人にお願いします」 と伝えるように心がけてます
レビューをされる側(レビューをお願いする側)
DOT YOUR I’S AND J’S
レビューを最優先してもらう代わりに、レビュワーには良いコードを送るようにしよう。 レビューをお願いする前に、自分で注意深くレビューしよう。 変更点に不足が無いように、また不必要なものが無いようにしよう。自分の能力の極限まで、属しているプロジェクトに適したコードを書こう。...
良いタイトル・説明を
タイトルを見れば、レビュー一覧の中から明らかに意味が伝わって分かりやすくなっていること。**レビュー依頼に書く説明は、変更に関するストーリーを書くべきだ。** (中略) **レビュワーに対して、特に何に気をつけないといけないか、分かりやすくなっているべきだ。**レビュワー自身も、どんな観点で見れば分かりやすくなるので、感謝してくれるだろうし、何より**レビューがより効果的・徹底的になる。**
すぐにコメントしてACKを
しっかりしたイベントループは、コードがレビューされているときには、さらに重要になる。つまり、レビュワーからコメントがあったら、すぐにACKするのに全力を尽くすこと。 またコメントが理解できなかったら、すぐに詳細な説明を求めること。レビュワーがレビューを投稿したら、次のアクションはあなたの番になる。...
下から目線のコードレビュー | steps to phantasien
レビューの良し悪し
...
良し悪しには定量的な指標だけでなく, 定質的な…というか主観的な指標もある. 要するにイラッとくるレビューもあれば目から鱗のレビューもある.
たとえば細かいコーディングスタイルの間違いや好み違いを指摘されるとまあまあ気分が悪い. すぐに慣れて何も感じなくなるけど, 本来ならできるだけ機械的なチェックで済ませたいもの. WebKit は検索会社由来の cpplint.py を改造した check-webkit-style が執拗に細々した間違いをみつけてくれるが, それでもスタイルや好みの指摘はなくならない. Go 言語の gofmt ツールもスタイル自転車置き場の不毛さを訴えている. コーディングスタイルだけでなく, 関数やクラスの粒度も好みの押し付けと意義深いレビューの境界にいると個人的には思う.
一方, 明らかなバグを指摘されると大変ありがたく思う. そして指摘が少なく, いつもすぐ LGTM をくれるレビュアは, ありがたいと同時に少し不安になる.
より抽象度の高い, 設計方針に関するフィードバック. これは内容によって毒にも薬にもなる. ただ基本的にエラい人 - 変更前のコードを書いた人や該当モジュールに詳しい人の意見は無視できない. それに詳しい人の指摘は大抵的を射ている. そんな指摘は, レビュイにとって主観的に良いレビューといえるだろう.
できるだけ細かい部分より、全体の方針や設計が変わるようなところから最初にコメントするように心がけています
どれくらいの緊急度なのかを RFC 2119 - Key words for use in RFCs to Indicate Requirement Levels に従って、[MUST] [SHOULD] [OPTIONAL] をつける文化はとても良かったです
Review Code Like a Pointy Haired Boss
問題領域やコードをよくわかってるプログラマがレビューすると意義深いフィードバックにつながる. 逆にいうと, 問題領域やコードをよくわかっていないプログラマによるレビューは(主観的に)ぱっとしない.
...
> 　腰が引けたレビューはこんなふうに進む: まず, 細かい揚げ足取りが増える. 些細なスタイルのあらをつついたり, 潔癖すぎるようなリファクタリングを言い出したりする. 心のどこかで LGTM を恐れているからだ. 発言が二転三転することも多い. 細部に対する理解が浅いせいで, 自分の的外れな指摘にあとから気づき主張がぶれる.
テストを書けとうるさいのも特徴. 理解の甘さをテストで埋め合わせようとする. 作りの良し悪しはよくわからんけどこれだけテストすればバグはなかろう, そんな言い訳がある. きちんとテストするのは悪いことではないけれど, プロジェクト全体の水準にあわない過剰さは無駄だ.
代理レビュアは最後までコードの正しさに確信を持てない. だから既存のコードを大きく変更するのを嫌う. 問題があったときにいつでも revert できるよう, 差分が小さく綺麗なパッチを求める. 結果としてデザインの整合性が損なわれることがある. あるいは過剰なリファクタリング要求とパッチの小ささが衝突し, パッチを細かくわけろと必要以上にうるさい.
腰が引けたレビューとは, 要するに保守的で形式的なレビューだ. 理解不足をプロセスで補おうとする.
思い当たる節がおおく、とても耳が痛いです :confounded:
#新人研修用資料 #ソフトウェアエンジニアの流儀