コードレビューで重視していること
レビュースラング用語
- LGTM
- FYI
- PTAL
コードレビューにおけるコメントの接頭詞
- must: 修正必須。
- want: 修正推奨。
- nits: 細かな修正。
- imo: 自分はこう思うが、いかがでしょうか?
- ask: 単純にわからないので教えてください。
![must](https://img.shields.io/badge/review-must-red.svg)
![imo](https://img.shields.io/badge/review-imo-orange.svg)
![ask](https://img.shields.io/badge/review-ask-blue.svg)
![nits](https://img.shields.io/badge/review-nits-green.svg)
![suggestion](https://img.shields.io/badge/review-suggestion-blue.svg)
プルリクエストへのコメントでは Text Blaze でラベルバッジをつけよう!
コミットメッセージの書き方で参考にできるもの
破壊的変更
ライブラリ作成時に破壊的変更を行う場合、ライブラリを利用しているプロジェクトのCIのビルドが失敗するように設計しなければならない。 ChangeLogの記入のみだと、その破壊的変更に気づかない可能性がある。
尋ねるな、命じろ
呼び出し側はメソッドを通じて処理を命ずるのみで、オブジェクトの内部状態やその状態に応じて呼び出し側の判断を行ってはならない。
読みやすいコード
チェック項目
- 複数の機能がある関数
- 不要なコードを削除していない
- 例外処理を行っていない
- マジックナンバーの使用
- 組み込み関数に代入
max
等、デフォルトで提供されている関数
- 境界値での動作
- オリジナリティのあるコード
- 揺らぎを減らそう
- ネストが深い
- クラスや関数を使用していない
- 識別子の命名が雑
- 動詞で始める
- x:
securelySendMessage()
- o:
sendMessageSecurely()
- x:
- 動詞で始める
- 実行時間、計算量
- 適切でないコメント
- 急いで開発する際に手が及ばなかった
- 普通の書き方に従わない場合の理由
- うまくいかなかった方法
- ミュータブル or イミュータブル
- 整合性の撮れてない並行、並列処理
知らないと恥ずかしいコードレビューで指摘されがちなポイント14選
else if より enum and switch
else if
は抜け落ちが生じてミスが生じやすいから気をつけよう。
チーム開発のノウハウ
改善デー・やさしさデー
後回しにされがちな問題を改善するための「改善デー」「やさしさデー」のご紹介
GitHub通知: 適切に設定しないと、自分にAssignされたコードレビュー等を見逃してしまうよ
READMEやテンプレートについて
コードレビューで気をつけること
コードを色分けして、良いコード、悪いコードの分析
GitHubのPRの作成とその後の議論方法
コードレビューで嫌われる人
新人プログラマをコードレビューで殺さない方法
円滑なコミュニケーションを育むための聴く技術
プルリクエストを小さくする技術
READMEに記載するべきこと