※お前の現場では再現性がないかも。

◆何のためにプルリクを捌くのか

幸せになるためです。

気に食わないパッチを弾くためではないです。

◆技集

▼ソフト&ウェット

コードを書いた人を責めてはいかんし、その人の努力を無に帰すような発言をしてはいけない。

単純な「名前イケてないから修正ね」「この書き方だめだから、こう書いて」「この処理まるまる意味無いですバカ」「お前はバカ」「金返せ」とか、吐き捨ててはいけないです。

動くコードではあるはずだから、それを及第点として認めてあげようや。お前の気持ちの問題だ。指摘すると同時にフォローが必要なんです。「この書き方もありなんですけど」という前置きが有ると無いとでは、受け取る側の士気が大きく変わる。

余計な一言を言うのではなく、相手の立場を思いやった発言しくされ。へりくだる必要はないけど、和らげる言葉をひとつ付け加えろ。

柔らかく、情あふれる感じに指摘しろ。パワーで捻じ曲げるのではなく、「こうしてみては」と誘導しろ。柔よく剛を制す。

動かない/ビルド出来ないコードだった場合は「は?」「は?は?は?」「お前っ…ん?…は?」くらい言いたくなるかもしれんけど、「まず動くところまでしてみましょう」くらい気の長いことを言え。

▼占術師

「~は~のほうで実装しておいて、~から蹴りやすくしておくのが吉です。」「ラッキーカラーはピンク。」みたいな感じで占う。

▼段階踏み

細かいプルリクにするように要請しれ。プルリクは細ければ細かいほどお互いに助かる。でかいプルリクは、見きれない。途方に暮れる気持ち。

どっかのコードをコピってぺって貼ったものである可能性もある。

でかいクソコードが飛んできてしまったときは、次の実装でプルリクの計画みたいなのを立ててもいい。「ここまででプルリク」みたいな線を引く。

▼白目

見なかったことにして通す。いつか誰かが苦しむ。

▼インタビュー

文字ベースでやり取りするのは限度があります。戦争になります。会話しましょう。多分大きなプルリクだろうから「読み合わせしますか」みたいな。その場合、相手にメモを取らせてね。

んで、不機嫌な物言いをしない。常にワクワクした感じに話せ。

▼勉強

中途半端にコード書けると、他人のコードの粗にキツく口出ししたくなる。でも、勉強し続けると自分の無知さに気付けるようになってくるので他人も許せるようになる。多分。

あと、回答に淀みがあると残念じゃね?レビュワーの知識が浅いと残念じゃね?だから勉強しなきゃじゃん?自分が「すげー知ってる人」にならないと人に教えらんねぇわ。引き出しを増やそう。

あるべき論が理由とともにパッと出せるようになって初めて一貫したレビューができるんじゃないのかね。それまでは日々精進です。頑張り給え。俺も頑張ります。

▼通しつつコメント

わりかし使える。収拾がつきやすくなる。

プルリクがマージされることはレビュイーにとって嬉しいし、逆にいえば弾かれることは悲しい。悲しくはないかもしれんが、小さなストレスになる。そういう意味で「一旦通すけど修正しておいて」っていうのは、次のプルリクが小さくなるしwin-win。

ずっと同じブランチでやり取りするのは苦痛だ。混乱するし。プルリクを弾くたびにstateが積み重なっているんだよ。理解できなくなってくる。

修正箇所が多い場合は新しいタスクとして要改良事項を列挙し、上から順番にプルリク投げるようにお願いすること。

▼スルー

俺は安易な三項演算子が嫌い。だけど、まぁスルーしたりしている。修正しようと思った時に瞬時に消せるものはスルーしていいと思っている。

いいレビューには具体的な指標が必要なんじゃなかろうか。

  • レビュイーが成長する
  • 設計が壊れない
  • レイヤー(抽象化)が壊れない
  • 一貫性が壊れない

とか。

人が成長しないレビューをしてもなんにもならん。楽にならない。幸せになれない。

可読性は多少壊れても問題ない…と言ってしまうと語弊があるんだけど、普通、可読性はさほど壊れない。設計さえ盤石であれば無駄なコードを消すだけで読みやすくなる。プルリクとか実装がずっとドロドロしているときは自分の成したソフトウェア設計を疑う。使用しているフレームワークを疑う。

▼対応しなくていいよ

「それでもいけるけど、俺ならそういう書きかたしねぇ」と思ったら、コメントに「【対応不要】」みたいなプレフィックスを付けてもいい。

その場合、かなり具体的に「こういう風に書けます」とレクチャーすること。あとメリットについて書いておく。「早期リターンは可読性が高まりますし変なメソッド書いていることに気づくことができますしテストが書きやすくなります。」みたいな。

ただ、嫌味な感じにしてはダメ。難しいかもしれんがな。

▼不確定宣言

自信がない指摘をしてもいい。「こういうことができるはず」という物言いは、時にあり。やりすぎると信頼揺らぎそうだけど。「調査してみてもらえますか」とお願いしてもいいだろう。

悩み続けてしまう人もいるからフォローは必要。進捗状況はこっちからプルしてあげろ。

▼緩和

物事を深刻にしてはいけない。ラフな面をみせるべき。コードが完璧になる日なんてこないし完璧にする意味もないんだから、完璧を目指してはいけない。

なんとなくでいいっすよーみたいな。

ごめんなさいとか悲しい言葉を言わせない。プルリクを楽しいことと思ってもらわないと。「いや、プルリク捌くので給料もらってんですから気にしなくていいです。」と言え。「早く通して欲しいときは何度もガンガン急かしまくってください」とか。

心理的な障壁をなくせ。

難しいことを頼むときは例えば「挑戦してみてもらっていいですか」とか、そういう物言いをしなさいな。相手を元気にしろ。エンハンスメント!

▼朱にまじれ

悪法であっても一貫性あったほうがいいってこともある。

「このプロジェクトではそうしてないので、ここでは書き方変えてください。」と要請する。「その書き方のほうが本当は好いんですが」のパターンと「お前自分で何やってるかわかってんの?」パターンがある。

▼TODO書いてね

アノテーションコメントを書くことを要請しろ。

「// TODO : 動いてない。つくる。」もあり。

動かすためにプルリクがでかくなるんだったら、動いてない&TODOコメント書いてある小さいプルリクをマージしたほうが114514倍マシ。テスト(ポリシー)がコケていてもいい。

  • TODO:やるやつ。いつかやるやつ。
  • FIXME:リリースまでに直すことが確定しているやつ。放置してはいけないやつ。「誰か直して」ではない。
  • HACK:「しょうがなくこうしている」という理由。「言語が、フレームワークが、ライブラリの設計が悪いんです」というハナシを書く。
  • NOTE(MEMO):仕様、または妙な仕様について書く。他には、「こうしないとコードがクッソ冗長になっちゃうからこういう形で書いてます。」という言い訳を書く。

基本的には上記で運用できるはず。他にもあるけど。

アノテーションコメントをハイライトしたり一覧したりするツールもあるから、それを導入してもいいかもね。

ただ、残作業をTODOコメントで管理できるわけはないよな。そもそもの話になるが、プロジェクトを管理していないんなら、しろ。ツールを導入してメンバーを教育しろ。ツールで管理できているのであれば紐づくチケットのURLとかコメントに埋めてもいいよ別に。ツール変えた時に参照できなくなったりするけどね。

あと、ただのコメント「// hogeをfugaする処理」みたいなのは書かせてはいけない。コメントが必要になるってことは読んで意味が理解できないコードだから。名前がおかしいとか英語がおかしいとか層の切り分けがおかしいとか。ただ、壮大な、大河ドラマが如きビジネスロジックをシコる時は必要になることも勿論ある。そのコメントの用途は主にCtrl + Fで検索してジャンプするためのアンカーであり、星をコードを継ぐものに対する理解の足がかりである。つまり、そういうコメントは段落の見出しとして書け。

▼機械的シビア

シビアにしたいルールについては、機械にやらせること。linterとかeditorconfigとか。

Azure DevOpsらへんのCIツール使えばプルリクのポリシーでビルド走らせるとか、テストコード走らせるとかできるよね。他のツールはしらんけど。ただ、「慣れないな」と思ったらやめること。チームと状況にマッチするやり方を模索してくれ。チームに向けて「模索しています」「もっといいやり方を提案してください」と言え。

ひとの書きっぷりについては、指摘しない。また、ツールと戦争してはいけない。なるべくデフォルト設定を貫いてくれ。

そうもいかないんだがな(暗黒微笑)

▼拒まぬ

聞かれたほうがマシ。黙っていられるより2000倍マシ。聞いてくれた、声をかけてくれたのだという感謝を常にしろ。邪険に扱ってはいけない。自分にできる全力で聞き、聞き終わったら親身になって回答しろ。

そうしてあげたほうがお互いに幸せだし、将来的に全体のパフォーマンスが向上する。

▼よい設計で渡す

よい設計ってのは、厳密な設計とは違う。開発しやすく、理解しやすく、覚えやすく、辿りやすい設計だ。(常人が)聞いたら/一度失敗したら理解できる設計が正しい。何度も間違えたり頭が混乱するような設計は、正しくても悪い。

ソフトウェア設計が腐っているとプルリクがでかくなる。永続化層の設計が狂ってるとプルリクがでかくなる。逆に、プルリクが小さくなる設計ってのがあるでしょう。大雑把に話すと、凝集度結合度のスコア(?)が低い場合、機能追加やら修正の変更が複数ファイルに飛び散る。

設計を腐らせる祟り神みたいな人もいる。現人神。コンクリの道を神通力により地雷の埋まったジャングルにしてしまう人がいる。Nullを扱わずに済むように慎重に設計したモジュールに謎のパッチ飛んできて何だろって思ったら極めてバカバカしい理由で機能を追加されてインターフェースからNull返却するように改悪されててぁぁアアアーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーーー???って叫んだことあるでしょう。プロジェクトに祟り神がいる場合はそれを注視し、定期的に様子を見に行くこと。祠を建て、供物を奉り、歌や舞踊を捧げ、荒魂を鎮めろ。

▼擬人化

コードを擬人化する。

「~というビルトインのメソッドがあるので、それを使って書いてあげてください。」みたいな。コードくんに何か良いことをしてあげるという動機にすれば、仕掛りやすい。

◆思うこと

プルリクという仕組みがあって良かった。コードを紙に印刷して赤ペン先生とかしたくねぇし。

いま、(メインじゃないプロジェクトで)プルリクを通さないで各自マージする開発をやってんだけど、酷いね。「技術力ない癖に変に自信持ってる年上(社員(俺も社員))」がいて、頭を抱えてる。独り言をほぼ言わない俺だが、思わず「なんでだ…」と呟きながら頭を抱えた。プルリクで見るクソコードと、既にマージされている(さっきマージされた)クソコードを見るのでは重みがぜんぜん違う。味わいがちがう。

俺が道筋を引いてREADME.mdも書いたんですけど、すっげぇ外れてくる。理解が出来ない外れ方をしてくる。小さいプロジェクトだから収拾つけようと思えば容易なんだが、逆に、こんな小さい単純なプロジェクトでもメタメタにコード汚せるものかって、驚いている。

そして何より、自信ありげにしていることにビビる。収まりどころってものを何一つ認識してないんだよね。汚さに対する嗅覚が死んでる。

自分がどんなにヤバいことをしているのかもわかってない。とんでもないコミットしてる。俺が一生作らないであろうコミットをサクっと拝める。「おぉ…」っつって、そうだな…ツチノコを見つけたような気持ちした。

新入社員ならしょうがない。でも年上だし。何をしてたんだろってホントに思う。技術できなくてもいいんだけど、自信満々なのがわかんねぇ。今後どうすんの?「収まった感じ」についての嗅覚が未だに培われていないのはヤベェ。ネットワーク知ってるわけでもないしコミュニケーション能力あるわけでもないし。

助けてあげようとしたらなんか嫌な顔してくるし。俺は別にほうっておいたっていいんだけど、それお前、大丈夫か?どうなりたいんだ?お前のキャリアが心配だよ。

みたいなね。小さいプロジェクトだからいいけど、つまり、コードレビューの仕組みは必要です。祟り神ってそのへんにありふれているから。

◆結論

そんな感じ。自分をアップデートしてレビュー力を高めろ。

なんか思ったらまた追記しとく。