妥協できないソニックガーデンのコードレビュー

Transcript

1. 妥協できないソニックガーデンのコードレビュー 2023.12.11 田中義人
2. 自己紹介 田中義人(yoshito) 神戸在住 ソニックガーデン プログラマ 2017〜 もうすぐ7年目 技術書典14向けに同人出版
3. みなさんから頂いた「参加理由」の事前アンケート レビューの観点やこだわりを知りたい 「妥協しないコードレビュー」がどこまで妥協しないのかに興味がある プロのコードレビューに興味がある
4. レビュー観点 セキュリティの問題はないか 課金関連の問題はないか 名前は適切か DRY原則に従っているか etc
5. 本日のお題 「妥協しないコードレビュー」とは？
6. ソニックガーデンのメンバーに聞いてみた
7. 似たコードがたくさんある場合にまとめられないか、DRYかどうか railsのバージョンが古い場合とかにもコメントする テストコードがないと気になる Rubocopの指摘を対応させる インデント・スペース・改行のレベルで違和感・統一感を指摘する 5行の変更のコードにコメント5個つくことある あとで泣きを見そう、と感じたコードはapproveしない（経験上、こういうことをやるとあとで痛い目を見るんだよな、と嗅覚が働いたコードはapproveしない） 設計がいけてなかったらそもそもまで戻る レビューが終わって無ければ本番にデプロイさせない 空行や変数名なども含めて、コードの全てに「なんとなく」がないか確認する 改行の意図もわからなかったら確認する コードレビューしにくい読めないコードは途中でレビューやめてやり直してもらう 変数名、関数名はとことん拘る レビュアーがパッと見て何をしようとしているコードなのか理解できる変数・メソッドのネーミングや設計・構造 1行１行に意志をもって何が起こるか理解できるコードになっているか コメント書いたら負け（コードの説明） 思想についても（好み）としてコメントする 意図がわからないコードになっていたらメンテできないので妥協したくない その処理はそこに書くのが正しいのか？ (責務) rubocop や formatter などにまかせられることは自動化して、レビューしたくない。 ソニックガーデンメンバーが考える「妥協しないコードレビュー」
8. 思想的な部分(concern をむやみに使うべきではないなど)はレビューに熱が入る。コードレビューは、お互いの思想を伝え合ったり、戦わせたりする場として重要なのかも。 一般的にメンテしやすいかどうか、読んでて気持ち悪くないか コメント書くなら意図を書いてもらう 命名の違和感は指摘する 相手がしらないかもしれないから異なる実装方法があれば提示する 機能を満たしているだけじゃなくて、メンテしやすいコードでもあること。適切なテストも書かれていること そのコードを5年メンテする気になるかどうか 引き継げそうかどうか 意味の通らない名前の指摘 （どちらが良いかは別として）自分ならこう書くというコメント 違和感はすべて伝える モヤッとでもコメントする コメントが３桁になっても躊躇しない 指摘が多くてもする。多すぎる場合は直接話しても指摘する。 でかいクラス・メソッドや、自分がしないと思う命名は指摘する データ設計レベルの間違いは正す 読み物として形になることまで考える（名前付け、設計・分割、フォーマットとか） なるべく複雑なロジックを避ける、必要ならテストやなぜそうしているかの説明があるか フォーマットのチェックなどは自動化しておく 意図が不明確な空行にコメントする ソニックガーデンメンバーが考える「妥協しないコードレビュー」
9. 機能が共通化してあっても、場所が適切でない場合は移動することを提案する あとで使うかも、的なやつは消すことを提案（YAGNI 意図が分かりにくいけど必要なコードにはコメントを添えてもらう コードに意図が込められているか 忘れた頃に理解できるか let/再代入許さない プログラムとしては動くとしても、将来のメンテナンス性を考えてシンプルな実装になるよう指摘する。素直な実装になっているかどうか そもそもの仕様から必要性を指摘したり、代替案を提示したりする 自分のロジックにはまらない部分は納得するロジックを説明してもらいたい レビューが終わったらロジックが揃った状態になりたい 適切に関数に切り出し 全体を通してロジックが揃っているか コードを読んだときの直感と、実際の処理の流れが一致してるか テストをちゃんと書いているか（このテストに意味ある？これでちゃんとテストの目的は果たせている？） 目的のわからないコードは意図を必ず聞く 書いたコードは１行ずつ説明できてほしい（自分で書いておいて「わからない」は許さない） 大きな手戻りになることがわかっていても、設計がイマイチだったら直してもらう 命名にはこだわる、しっくりこなければ一緒に考える コメントがなくてもコードを読めば理解できるのが理想 自分が書くことを想像した時、しっくりくるところまでコメントする。 「今更」とか考えず、気づいた話は設計でもコードでも全部言って議論する ソニックガーデンメンバーが考える「妥協しないコードレビュー」
10. 妥協しないコードレビューとは? ChatGPTを使って要約 将来にわたって面倒を見ることができるか コードを読めばすべてがわかるようになっているか 思ったことすべてフィードバックする 単なる品質チェックではなく、技術的な思想を共有・議論する重要な場となっている
11. 実例を見てみましょう
12. コメント数が3桁になっても躊躇しない、納得するまで議論する
14. 空行にもこだわる
15. コメントにもこだわる
16. 思想についても（好み）としてコメントする
17. 自分のことは棚に上げてでも思ったことは伝える
18. 代替案がなくても違和感は伝える
19. 妥協できないソニックガーデンのメンバー プログラミングに対して妥協ができない 自分が書くときも妥協できないので、レビューのときも妥協できない
20. でも決して怖くは無い みんな良いコードを書くために楽しんでます
22. 結論
23. 妥協しないコードレビューとは？ プログラミングに妥協できない人たちによる、最高のコードを追求し続ける活動
24. 妥協しないコードレビューの事例をもっと知りたい方は
26. ソニックガーデンのジムやキャンプに参加すれば「妥協しないコードレ ビュー」を体験できます
27. ご清聴ありがとうございました