先週日本にインドネシアのプロダクトマネージャーが来た時に「レビューに時間がかかりがち」「結果として開発のリードタイムが予測しづらくなっている」という悩みを相談してくれた。そのときに「コードレビューを会話しながら行う取り組み - Hatena Developer Blogとかどうですかね、日本のチームでも “レビュー会” ってのをやっていますよ」と伝えようとしたが日本語の記事を渡すのも気が引けたので「あとでまとめておく」ってことにして社内向けに “Encouragement of Review Meeting” という snippet を書いた。
これを日本語でも書き直してみる。
背景
最近プロダクトマネージャーと開発者間でレビューに関してこんな感じの問題があるらしい。
~ ある日 ~
マネージャ「進捗どう？」
開発者「だいたい終わってレビュー待ちの状態かな」
マネージャ「オッケー」
~ 別の日 ~
マネージャ「どう？Pull Request はマージされた？」
開発者「いや、それがまだで…。もう一回 ping してみる」
マネージャ「そうか…どれぐらいかかりそう？」
開発者「うーん、わからん…」
~ 以下、ループ ~
開発者による見積もりがすぐに元々の数字から変わったり、リリース日がずれたり、誰にとっても嬉しくない状況である。
一方、日本の開発者は複雑だったり大きすぎたりする Pull Request を作ってしまったときには “ペアレビュー” や “レビュー会” を設けており、今のところうまく機能している。
前提
そもそもレビューのリードタイムが長くなるのは Pull Request がでかいか複雑か、それに見合う説明が足りてないか、だ。
まず開発者が意識すべきなのは
- Pull Request を適切なサイズに分割すること
- 充分な説明、図、wiki などを書くこと
とはいえでかい Pull Request を作らざるを得ないこともあり、得てしてそれは長い間放置されがちである…。
詳細
そうした状況を避けるため、日本の開発者は “レビュー会” というのをやっている。
- レビューを求める開発者（レビュイー）はまず Pull Request （必要なら wiki なども）を前もって用意し、ミーティングを予約する
- 全員が主体的に参加できるよう、レビュワーの数は2 ~ 3人がベストだと思う
- レビュイーはミーティングで Pull Request や wiki や図などを見せながら説明する
- レビュワーはいつでも何でも質問して良い
- たまに激しい意見が飛び交うかもしれないが落ち着いて欲しい
- 質問されたこや議論の内容は GitHub の review comments として残す
- ミーティングが終わったらコードを修正する
- 結構な量の修正になったとしても指摘内容や実装意図を理解しているので再レビューはかなり簡単
Note
- 全ての Pull Request がこうした取り組みを必要としているわけではない
- “ペアレビュー” も “レビュー会” もコミュニケーションツールだということを忘れないこと。普段レビューを円滑にするために行っていることをそのままやるとよい
- レビュイーは対面だからとか時間を取ってしまうからと気兼ねせずレビュー依頼や質問をすること
- レビュワーは良いところは褒めつつレビューされたし
終わり
Original post:
Background
I recently heard some @quipper/product-managers (or even developers) are troubled with reviewing time. Is the situation like below?
~ One day ~
Manager: “How is your work going?”
Developer: “Almost finished, but… it still needs to go through review.”
Manager: “Understood.”
~ Another day ~
Manager: “Did your PR get merged?”
Developer: “Ah, not yet. I’ll ping them again.”
Manager: “Hmm, I see. How long does the review take?”
Developer: “Umm, not sure.”
~ To be continued… ~
It’s so frustrating for everyone. Estimation by a developer easily varies from the original one, and its release date does.
On the other hand, Japanese developers sometimes have kind of “pair reviewing” or “review meeting” for a complicated (large) pull-request in person. So far, it works so well that I’d like to share the way.
Premises
Most of the review time issues are caused by a large/complicated pull request, and lack of adequate description.
So first of all, developers should
- split pull-requests into adequate size ones if possible
- write an adequate description, chart, wiki etc. for a feature they develop
Even though the idea above is right, we are sometimes compelled to implement a large/complicated pull request. And it will have been neglected for a long time…
Details
To avoid such situation, Japanese developers sometimes have “review meeting”.
How?
- A reviewee who needs a review prepares PR and wiki (if necessary) beforehand, and book a review meeting
- 2 or 3 is a good number of reviewers. If it’s too much, all of them might not actively join a discussion.
- The reviewee explains his/her implementation by showing PR, wiki, chart etc.
- I know some devs are quite good at to write a chart.
- A reviewer can ask/discuss anything anytime in the review meeting
- Be calm, it’s not an interview.
- Questions/Discussion points are noted as a review comment on GitHub
- If we skip this step, some points will be missed.
- After that, the reviewee reflects the content of the review meeting
- Even if there are so many fixes, it’s much easier to re-review since reviewers know why those changes are needed.
Note
- Not all of pull-requests requires “review meeting” and “pair review”.
- They are just communication tools, so
- If you are a reviewee, don’t be afraid to ask a review.
- If you are a reviewer, it’s quite encouraged to pay a high compliment to one’s code.
END
