Slide 1

Slide 1 text

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

Slide 2

Slide 2 text

自己紹介 田中義人(yoshito) 神戸在住 ソニックガーデン プログラマ 2017〜 もうすぐ7年目 技術書典14向けに同人出版

Slide 3

Slide 3 text

みなさんから頂いた「参加理由」の事前アンケート レビューの観点やこだわりを知りたい 「妥協しないコードレビュー」がどこまで妥協しないのかに興味がある プロのコードレビューに興味がある

Slide 4

Slide 4 text

レビュー観点 セキュリティの問題はないか 課金関連の問題はないか 名前は適切か DRY原則に従っているか etc

Slide 5

Slide 5 text

本日のお題 「妥協しないコードレビュー」とは?

Slide 6

Slide 6 text

ソニックガーデンのメンバーに聞いてみた

Slide 7

Slide 7 text

似たコードがたくさんある場合にまとめられないか、DRYかどうか railsのバージョンが古い場合とかにもコメントする テストコードがないと気になる Rubocopの指摘を対応させる インデント・スペース・改行のレベルで違和感・統一感を指摘する 5行の変更のコードにコメント5個つくことある あとで泣きを見そう、と感じたコードはapproveしない(経験上、こういうことをやるとあとで痛い目を見るんだよな、と嗅覚が働いたコードはapproveしない) 設計がいけてなかったらそもそもまで戻る レビューが終わって無ければ本番にデプロイさせない 空行や変数名なども含めて、コードの全てに「なんとなく」がないか確認する 改行の意図もわからなかったら確認する コードレビューしにくい読めないコードは途中でレビューやめてやり直してもらう 変数名、関数名はとことん拘る レビュアーがパッと見て何をしようとしているコードなのか理解できる変数・メソッドのネーミングや設計・構造 1行1行に意志をもって何が起こるか理解できるコードになっているか コメント書いたら負け(コードの説明) 思想についても(好み)としてコメントする 意図がわからないコードになっていたらメンテできないので妥協したくない その処理はそこに書くのが正しいのか? (責務) rubocop や formatter などにまかせられることは自動化して、レビューしたくない。 ソニックガーデンメンバーが考える「妥協しないコードレビュー」

Slide 8

Slide 8 text

思想的な部分(concern をむやみに使うべきではないなど)はレビューに熱が入る。コードレビューは、お互いの思想を伝え合ったり、戦わせたりする場として重要なのかも。 一般的にメンテしやすいかどうか、読んでて気持ち悪くないか コメント書くなら意図を書いてもらう 命名の違和感は指摘する 相手がしらないかもしれないから異なる実装方法があれば提示する 機能を満たしているだけじゃなくて、メンテしやすいコードでもあること。適切なテストも書かれていること そのコードを5年メンテする気になるかどうか 引き継げそうかどうか 意味の通らない名前の指摘 (どちらが良いかは別として)自分ならこう書くというコメント 違和感はすべて伝える モヤッとでもコメントする コメントが3桁になっても躊躇しない 指摘が多くてもする。多すぎる場合は直接話しても指摘する。 でかいクラス・メソッドや、自分がしないと思う命名は指摘する データ設計レベルの間違いは正す 読み物として形になることまで考える(名前付け、設計・分割、フォーマットとか) なるべく複雑なロジックを避ける、必要ならテストやなぜそうしているかの説明があるか フォーマットのチェックなどは自動化しておく 意図が不明確な空行にコメントする ソニックガーデンメンバーが考える「妥協しないコードレビュー」

Slide 9

Slide 9 text

機能が共通化してあっても、場所が適切でない場合は移動することを提案する あとで使うかも、的なやつは消すことを提案(YAGNI 意図が分かりにくいけど必要なコードにはコメントを添えてもらう コードに意図が込められているか 忘れた頃に理解できるか let/再代入許さない プログラムとしては動くとしても、将来のメンテナンス性を考えてシンプルな実装になるよう指摘する。素直な実装になっているかどうか そもそもの仕様から必要性を指摘したり、代替案を提示したりする 自分のロジックにはまらない部分は納得するロジックを説明してもらいたい レビューが終わったらロジックが揃った状態になりたい 適切に関数に切り出し 全体を通してロジックが揃っているか コードを読んだときの直感と、実際の処理の流れが一致してるか テストをちゃんと書いているか(このテストに意味ある?これでちゃんとテストの目的は果たせている?) 目的のわからないコードは意図を必ず聞く 書いたコードは1行ずつ説明できてほしい(自分で書いておいて「わからない」は許さない) 大きな手戻りになることがわかっていても、設計がイマイチだったら直してもらう 命名にはこだわる、しっくりこなければ一緒に考える コメントがなくてもコードを読めば理解できるのが理想 自分が書くことを想像した時、しっくりくるところまでコメントする。 「今更」とか考えず、気づいた話は設計でもコードでも全部言って議論する ソニックガーデンメンバーが考える「妥協しないコードレビュー」

Slide 10

Slide 10 text

妥協しないコードレビューとは? ChatGPTを使って要約 将来にわたって面倒を見ることができるか コードを読めばすべてがわかるようになっているか 思ったことすべてフィードバックする 単なる品質チェックではなく、技術的な思想を共有・議論する重要な場となっている

Slide 11

Slide 11 text

実例を見てみましょう

Slide 12

Slide 12 text

コメント数が3桁になっても躊躇しない、納得するまで議論する

Slide 13

Slide 13 text

No content

Slide 14

Slide 14 text

空行にもこだわる

Slide 15

Slide 15 text

コメントにもこだわる

Slide 16

Slide 16 text

思想についても(好み)としてコメントする

Slide 17

Slide 17 text

自分のことは棚に上げてでも思ったことは伝える

Slide 18

Slide 18 text

代替案がなくても違和感は伝える

Slide 19

Slide 19 text

妥協できないソニックガーデンのメンバー プログラミングに対して妥協ができない 自分が書くときも妥協できないので、レビューのときも妥協できない

Slide 20

Slide 20 text

でも決して怖くは無い みんな良いコードを書くために楽しんでます

Slide 21

Slide 21 text

No content

Slide 22

Slide 22 text

結論

Slide 23

Slide 23 text

妥協しないコードレビューとは? プログラミングに妥協できない人たちによる、最高のコードを追求し続ける活動

Slide 24

Slide 24 text

妥協しないコードレビューの事例をもっと知りたい方は

Slide 25

Slide 25 text

No content

Slide 26

Slide 26 text

ソニックガーデンのジムやキャンプに参加すれば「妥協しないコードレ ビュー」を体験できます

Slide 27

Slide 27 text

ご清聴ありがとうございました