// 別に、プルリクレビューの前にレビューしてもらっていいんだからね
https://jflute.hatenadiary.jp/entry/20170630/reviewbefore
こちらの続編のようなものです。
このブログの「2. 実装してる最中レビュー」
にて...
「どういうタイミングで出したらいいのか?」
という問い合わせを、1on1で頂きました。ということで、そのときに思いついたものをまとめてみます。
A. ラフスケッチの実装レビュー
まず...
ラフスケッチ的な実装をして見てもらう
と良いのではないでしょうか?
おおまかな実装の方向性を提示
ざっくり全体の流れを実装して、実装に時間の掛かりそうな込み入った部分は保留。例外ハンドリングなども保留。
そして...
パッケージやクラス名、メソッド名や処理の順序、フレームワークの使い方、利用するライブラリなど、「全体の構造に関わる部分」だけをレビューしてもらいます。
o この現場のポリシーに適した実装になっているか?
o ちぐはぐな構造の実装になっていないか?
o あれこれ命名がへんてこりんじゃないか?
o フレームワークの使い方が間違ってないか?
o 利用するライブラリを間違ってないか?
「おおまかな実装の方向性」を先に提示して、「そもそも方向性が全然違う」とかあったら早い段階で指摘してもらうことで大きな手戻りを防げます。
そのためには?todoコメント
そのためには、todoコメント (保留コメント) が欠かせないでしょう。
レビューワーが
「あれ、こんなんでいいの?」
レビューイーが
「いや、そこはまだ保留で...」
ということになるから実装途中のレビューが成り立たないもので、飛ばしたところは遠慮なくtodoコメントを入れておきましょう。
List memberList = selectList(cb -> { cb.setupSelect_MemberStatus(); // TODO jflute 細かい条件は後で (2018/10/10) }); if (isYabakunai(memberList)) { yabaiShori(memberList); } try { tondemonaiShori(memberList); } catch (RuntimeException e) { // TODO jflute いま雑 (2018/10/10) // TODO jflute リカバリもあと (2018/10/10) } ... private boolean isYabakunai(List memberList) { return false; // TODO jflute 詳しい判定あと (2018/10/10) }
そのためには、todoコメントを入れるのが億劫にならないようにすることが大事です。まず、敬語とかどうでもいいです。最終的には全部消しますから。そして、エディターの機能で補完できるようにしておくと良いでしょう。
実際に、jfluteの関わっているいくつかの現場では、IDEの拡張設定で _todo (実質、_to くらいでOK) で補完すると...
// TODO [名前] (カーソルここ) (現在日付)
というように、名前と日付が自動的に入ったtodoコメントが補完されるようにしています。導入研修のときに必ず教えるので、現場のほとんどの人に覚えてもらっています。(Eclipse, IntelliJ 両方で同じものを用意)
保留コメントが、残ったままにならないようにするという目的もありますね。名前と日付を入れることで気付きやすく追跡もしやすいようにと。
B. 動作確認の前の実装レビュー
しっかりと時間をかける(最終的な)動作確認の前に、見てもらうと良いのではないでしょうか?
テストファーストじゃなく実装ファースト (テストセカンド?テストレイター?) であれば、これから UnitTest のコードを書く前に。まあ、テストファーストであっても、実装に行く前にテストコードを見てもらうとか!? (応用できるパターンはいくらでも)
動作確認の手戻り
これはすごく単純です。盛大にテストをしてから見てもらって「違うよ」ってなって、根本的な方式とかもろもろ変わって「あー、動作確認の手間が無駄だったー」とかにならないように。
手戻りって、書く作業だけじゃない
のです。
そのためには?レビューの観点
そのためには、レビューの観点を伝えることが欠かせないでしょう。
「はい、見てください」では、レビューワーが勘違いして動作確認が取れていることを前提にレビューしてしまい、「これ動かないんじゃない?」とか無駄なレビューになってしまう可能性があります。それでは本末転倒です。
「動作確認はしていません」では、レビューワーが「はぁ?」と言ってしまいます。
「まだ、動作確認の前で、全体的な実装の方向性とか、命名など動作可否とは無関係の実装上の不備をレビューして頂きたいです」と言えば、レビューワーも観点が明確になって「ふむ、したら、これこれこうで、あれあれあう」と視点を限定してコードを見ることができます。
...
ちなみに、この観点を伝えるのは、別に最後のプルリクでも同じと言えば同じです。「特にこの辺を重点的に」とか「ここがすごく不安です」とか、今のコードに対する作り手のニュアンスを伝えることで、より安全なフィードバックがもらえるわけです。レビューワーの負荷を下げることにもつながります。
レビューワーも嬉しい
段階的にレビューするのは、レビューワーも嬉しい
ものです。
もし積み上がったものを一気に見るとなると、一つのレビューというタスクが非常に重く、スケジュールの中に組み込みづらいですし、それなりの集中力を要します。
それこそ手戻りが発生した場合、実はレビューワーも二回レビューをすることになるわけです。
手戻りは実装している本人だけじゃない
のです。
段階的であれば、観点を絞ったレビューになるので、一つ一つのレビューに対するプレッシャーは低くなります。「今回は、この観点だけで見ればいいのか」と視点を絞って見られるというのは大きくレビューワーの負荷を下げるものです。例えばラフスケッチの時点で一度コードを目を通しておけば、レビューワーが全体像を把握しやすく、完成時のレビューでもコードを読む際の見通しが良くなります。
考えるきっかけ
まさしくこういうことを気にしていた若者との1on1でとっさに出たものだけ書いてありますので、ざっくりニパターン出してみましたが、もっともっとタイミングはあるかも知れません。
みなさんも色々と工夫してみると良いのではないでしょうか?
それにしても、1on1からブログを書くきっかけが生まれるなんて素敵だなって(^^。(ブログ書くよーとは言ってありまっす)
だにえるありがとう。