コードレビューについて

ここで紹介されている、構成管理システム(VCS)でのレビューコントロールがとてもエレガントだと思う。

レビューのために bug tracker や task management system を使うのはあまり良いとは思いません。 レビューでは非常に細かい点が議論されることがあり、これが仕事のタスクの一チケットに相当するとはとても思えないからです。 例えば、この変数名は短すぎて良くわからない、といったことのために bug tracker をブラウザで開き、チケットを切る、やってられるでしょうか。レビューは VCS に記録されていますから、コードベース上に現在どれくらいのオープンなレビューがあり、どれが緊急を要するかなどはレビューコメントに一定の書式を入れ、簡単な VCS の検索を行うことでほとんど実現可能です。
コードレビューについて - Oh, you `re no (fun _ → more)

• 潔癖症・完璧主義で、あらゆるコードの修正を記録したがる人が時々いるけれども、使いどころは考えたほうが良い。早い段階からBTSへの登録を強制してもあまりうまくいかない。
• 構成管理システム(VCS)に最大限の情報が集積される、というのが最も合理的だと思っている。
  • コードそのもの、及び修正履歴とコミットログに出来るだけ情報があるといい。
  • 別にアジャイルソフトウェア開発宣言を意識して文書を軽視しているわけではない。何らかの緊急的なトラブルが発生する事態を想定した時、構成管理に多くの情報があると安心なのだ。
  • プログラムの設計は意思決定の履歴だと考えているので、この履歴がVCS上で追えるのは大きな利点だと思う。

修正前のコードをコメントアウトして残す悪習について

バカバカしい話ですが、ソースコードをSubversionなどでバージョン管理しているにもかかわらず、未だ修正前をコメントアウトして残す習慣は残っているところも多々あります。
変更前をコメントアウトして残す習慣は未だ根強い - 日々常々

プロジェクトの制約にもよるけれども、私は問答無用で削除してしまう派。消すなと言われても、まずは徹底抗戦して消すことを調整する。

• このような慣習の根源は、「構成管理の環境が充分に整っていなかった」時代の名残りであったり、「机上デバッグの文化」の名残りなのだと思っている。
  • 「構成管理の環境が充分に整っていなかった」時代は、例えば個々の担当者が自由にコードの全世代の修正履歴を確認することはできなかったので、過去のコメントが参考に残っていることは意味があった。しかし現在は、構成管理システムによって、個々の担当者は過去のコードに容易にアクセスできるし、現在との差異も簡単に確認できる。だから、修正前コードを残すことにはあまり意味は無い。
  • 開発資源（端末や計算機など）が限られていた時代には、少ない資源を有効に活用するために机上デバッグを行なっていた。この際に参考とする意味で修正前コードをコメントアウトして残しておくという意味はあったかもしれない。しかし現在は端末やサーバ機器などの開発資源は潤沢で、ボトルネックは人間という時代である。机上デバッグも実施していないことから、修正前コードを残すことにはやっぱり意味は無い。
• こういった経緯を踏まえて「修正前コードを抹消する」ことは充分に可能だと考えている。しかし問題はソフトウェア開発をめぐるこういった歴史的経緯をどれだけ語れるか、という事でもあると思う。ソフトウェア開発の技術進歩を目の当たりにした世代が、時代の変化を捉えて悪習を断つことをしないといけない。次の世代に無意味な慣習を継承しないことについてもっと考える必要がある。

古くからの習慣を打ち破るのは容易ではない。古い習慣に気づくのはそれに輪をかけて難しい。古い習慣を捨てるための最初の一歩は、自分のやり方が時代遅れだということに気づくことだ。これが最も難しい。同じくらい難しいのは、実際にそれを捨てることだ。メンタルモデルや思考パターンは、長年にわたり多大な代償を払って形成され、磨かれてきたものだ。だから軽々とは捨てられない。
アジャイルプラクティス 達人プログラマに学ぶ現場開発者の習慣 7.時が来たら習慣を捨てる(P36)

構成管理警察のこと

自分がプロジェクトマネージャをやる（それなりの規模の）プロジェクトのメンバーは、ときどき「構成管理警察」からのメールを受けることになる。PM(私)が構成管理システムの1日のコミットログとコメントをチェックしており、気になる点があると個別担当者に確認をするのだ。その時、自分のことを構成管理警察と呼んでいるというわけである。もちろん全てのコードの修正内容を見ることは難しい。けれど、「目を光らせている」ことをアピールする程度には見るようにしている。なお、別に叱責するような指摘の仕方はしない。「これ見たんだけど、次はこうしたほうがいいんじゃない？なぜなら・・・」と意見交換するようなニュアンス。

• 網羅的に全てのコードやログは見れないし、見ない
• プロジェクトの現在の話題のキーワード(メールや朝会などで話題に出たもの)は重点的に見る
• 「暫定」とか「仮」とか気になるキーワードリストを用意しておいて、キーワードにヒットする怪しいコメントなどについてはツッコミをいれる
• コミットコメントが無いとか、ルールを逸脱しているものについてもチェックする
  • ただし修正までは求めず、次回から気をつけるように注意する程度

以前にチーム内でやる進捗会議はムダという記事を書いたことがあるのだけれども、構成管理ツールに習熟すると、静かに眺めているだけでプロジェクトの健康状況を知ることができる。だから、Excelのように、構成管理ツールをプロジェクトマネージャはよく理解しておいたほうがいい。ちなみに、GUIツールよりはコマンドライン操作のほうが自由度が高いので、コマンド操作一式について習熟することを勧める。

個人的にはこの2冊が鉄板。ツールに関する書籍は、利用しているものにあわせて選んだら良いと思う。