最近めっきり便利屋さんみたいなポジションになってしまい、サーバーにしろアプリにしろ「プルリクお願いします」と言われることが多くなってきました。
僕もスーパーなエンジニアでは全くないのであれこれ試行錯誤しながらコードを見ているのですが、今回はそんな中でやられると困っちゃう（オブラート10枚くらいで包んだ表現）ことをまとめます。
ちょっと困っちゃうやつ
- プルリクの説明がコミットログ（しかもテキトーに書いたやつ）の羅列
- プルリクのタイトルがない
- 改行コードが変わったのか、ファイル全体が差分になる
- 明らかに不要なコメントアウトが残ってる
- 個人的なデバッグ用のコードが残ってる（sysoutとか）
- 指摘された部分を修正した際、同じ内容の他の箇所が直っていない
困っちゃうやつ
- コードに明らかに実装漏れ（空のメソッドとか）があったので指摘したら、「あ、それ別のブランチで対応予定です」って言われる
- プルリクの説明にない対応が一緒に入っている
とても困ったちゃん
- 時間かけていろいろ指摘したら、「プルリク出せば完成だと思ったので修正時間は確保してませんでした」って言われる。なんなら「いちいちうるせーなー早く承認してよ」みたいな反応される。
- そもそもの設計から指摘したくなるボリュームのソースコードをいきなりプルリクされる
解説
- プルリクの説明がコミットログ（テキトーに書いたやつ）の羅列
- プルリクのタイトルがない
このあたりはプルリク出すときに、きっと流れ作業のようにプルリクボタンを押しているのだと思います。
チェックする人はコンピューターではないので、コードを見れば全てを理解できるわけではありません。ぜひ人間の言葉でこのプルリクが何を表すのかを書いておいてほしいところです。
後々見返すときにもとても重要です。
- 改行コードが変わったのか、ファイル全体が差分になる
- 明らかに不要なコメントアウトが残ってる
- 個人的なデバッグ用のコードが残ってる（sysoutとか）
プルリクを出す前に、必ず自分がレビュワーになったつもりで一度変更を全て確認してほしいと思います。
この手の見返せば自分で気づける系の指摘をするのはとても時間の無駄ですし、場合によってはそれがミスなのか意図的なのか、意図的だとしたらどんな意図があるのか、などを推察したり確認したりと余計な手間がかかってしまいます。
- 指摘箇所を修正した際、同じ内容の他の箇所が直っていない
これはちょっと「こことここも以下同文で直しておいてね」と一言言っていないレビュワーにも原因があるので何とも言えないのですが、一応気を付けてほしいこととして載せてみました。
- コードに明らかに実装漏れ（空のメソッドとか）があったので指摘したら、「あ、それ別のブランチで対応予定です」って言われる
ならそうどこかに書いておいてください。
上記と同様、何か理由があるのかと思って余計な労力を使ってしまいます。
ちゃんと事情があれば、それを踏まえてチェックすることができます。
- 時間かけていろいろ指摘したら、「プルリク出せば完成だと思ったので修正時間は確保してませんでした」って言われる。なんなら「いちいちうるせーなー早く承認してよ」みたいな反応される。
何のためにプルリクしているのかを考えてほしいところです。
プルリクは偉い人が書類にハンコを押す作業ではありません。複数人の目で見ることによって、ひとりで開発しているときには気づけなかった観点を入れ、プロダクトをより良いものにするための工程です。流れ作業のように確認して終わり、ではありません。
指摘が入るのは当たり前ですし、そこから議論を重ねて「なんでこう書いたのか」をチームとして説明できるようになって初めて最終的なリリース物として世に出すことができるソースコードになります。
もし何らかの事情があって、指摘に対する議論や修正の時間がないほど切羽詰まっているのであればそう言ってください。それならTODOを残しておいて後で修正する等の対処ができます。
- そもそもの設計から指摘したくなるボリュームのソースコードをいきなりプルリクされる
どこからツッコめばよいのか分からなくなります。事前（コーディング前）に設計レビューの場を用意してください。
まとめ
プルリク見てると実感するのですが、プルリク見るのって相当エネルギーを使います。
本当に一行一行、一文字一文字について変なミスがないか、そのコードで1年後に問題が出ないか、人が変わっても大丈夫か、などいろいろなことを考えながらチェックします。なんなら自分でコーディングするときより気を遣ってます。数行の変更や追加でも時間を使います。それでもプロダクトをよりよくするためにリソースを確保するわけです。
そのため今回挙げたような、そのリソースが無駄になったり余計なリソースを使わせたりさせられることをされると「困っちゃう」わけです。特にプルリクを単なる承認作業だと思われるのが一番ツラいです。
全てはより良いプロダクトを世に出し、ユーザーも開発者も幸せになるための大事な工程ですので、プルリク出す方も見る方も少しの気遣いで建設的に進めたいと思う今日この頃です。
追記 (2016/07/27)
この記事だけだと僕がプルリク嫌いな人みたいな印象を与えてしまうような気がしてきたので、プルリクでやられると嬉しいことまとめ という記事を書きました。
よろしければ併せて読んでみてください。