Code Review FAQ

コードレビューの目的は何ですか？

コードレビューは、パッチの設計と実装について検証を行うための、基本的な機構です。沢山のハッカーたちが Mozilla の様々なモジュールに対して行う設計と実装を、常に一貫したレベルのものにするために役立っています。私たちは現在、二段階のレビューを行っています。「レビュー」と「スーパーレビュー」です。

もちろんコードレビューは即座に行えるものではなく、このシステムにはある程度の待ち時間が伴います。私たちは、レビュアーやスーパーレビュアーが彼ら自身のハッキングを行うのを妨げずに、レビューの待ち時間を減らす方法を常に模索しています。私たちのシステムは完璧ではなく、将来も完璧にはならないでしょう。私たちは現在もシステムを発展させている途上であり、もし良い方法があるのなら提案してください。

私が書いたコードのレビューを行うのは誰ですか？

あなたは、コードがチェックインされる予定のモジュールの所有者、あるいは指定された「ピア (peer：同格の人)」から承認 ("r=[name]") を貰わなくてはなりません。 あなたのコードが複数のモジュールに影響を及ぼす場合には、影響を受けるモジュールのオーナーあるいは指定されたピアから "r=[name]" を貰わなくてはなりません。 私たちはこの点に関して合理的であるよう心がけていますので、モジュール所有者全員の承認が必要な場合についての厳密なルールはありません。例えば、文字列クラスの変更や、多数のモジュールで表示されるテキストの変更のように、ツリー全体にまたがる変更の場合、必ずしも全てのモジュール所有者のレビューを必要とはしません。

上記以外の人に、追加のレビューを依頼しても構いません。

レビュアーが検討するものは何ですか？

レビューでは、パッチの設計と実装、対象となる問題の解決における有用性、そしてモジュールへの適合性に重点が置かれます。レビュアーは、その問題の領域における専門知識を持った人物であるべきです。また、レビュアーは、その領域以外の専門知識を利用して、他の改善案の可能性を指摘することもできます。 レビュアーがコードの改善に関して行うコメントに、固有の制限はありません。

スーパーレビューとは何ですか？

「スーパーレビュー」という名称は、本当は好ましくありません。「インフラストラクチャ・レビュー」または「インテグレーション・レビュー」という名称のほうが、多分正確であり、「スーパー」という魅力的な名称が招く誤解を避けることが出来るでしょう。しかし、この名称は長い間使われてきましたし、当面は「スーパーレビュー」という名称が使用され続けるでしょう。

スーパーレビューでは、単なるレビューとは異なる問題に重点が置かれます。 Mozilla コードベースの幅広い範囲において、パッチがうまく適合するかどうか、ということです。例えば、以下のようなポイントが論点となります。

• API の使用に関して
• XPCOM の使用に関して
• Mozilla のポータビリティ・ガイドラインに忠実であるか
• 複数のモジュールへの影響
• Mozilla のコーディングの慣例

スーパーレビュアーには専門知識が必要ですか？

いいえ、そのパッチが対象とする分野の専門知識をスーパーレビュアーが持っている必要はありません。 私たちは、スーパーレビュアーに対して、専門知識を持たない領域でレビューを行ってくれるよう頼んでいます。そうしないと、負荷のバランスが取れなくなるのです。専門知識を持ったスーパーレビュアーは、その領域の範囲内のパッチ全てにレビューを行わなければならないと感じるでしょう。これは燃え尽き症候群への道です。さらに、スーパーレビューを行うために重要であるのにも関わらず、誰一人としてその領域に関する幅広い知識を持たない、あるいは持ちたくない領域もあるでしょう。

もちろん、スーパーレビュアーが専門知識を持っていて悪いということはありません。私たちは彼らがスーパーレビューにおいてその専門知識を使ってくれることを望みますし、コードの改善に関してスーパーレビュアーが行うコメントに、固有の制限はありません。専門知識を持ったスーパーレビュアーは、レビューがどの程度複雑になりそうかを迅速に評価し、スーパーレビューをより早く完了させ、そのパッチに対する包括的な評価を与えることができるでしょう。例えて言えば、どんな名シェフでもあなたの料理を改善できますが、あなたがクレオール料理を作ろうとしているのであれば、クレオール料理の名シェフならなお良い、ということです。

ですので、専門知識は付加価値にはなりますが、必須のものではありません。スーパーレビュアーが希望すれば、そのレビューが、専門知識には基づかず、包括的な問題についてのみのレビューであることを注記することができます。

スーパーレビューのドキュメント でスーパーレビュアーと専門領域が関連づけられているのは何故ですか？

既に述べた通り、専門知識を持ったスーパーレビュアーはより早く、包括的なレビューを行うことが出来ます。あなたの作業範囲に専門知識を持つスーパーレビュアーがいる場合には、まずその人に連絡を取ってください。できるものであれば、デラックスバージョンのスーパーレビューを受けましょう。

専門知識を持ったスーパーレビュアーがいなかったり、また、いたとしても彼らが忙しすぎるような場合には、別のスーパーレビュアーをあたってください。強力なレビュアーとできるだけ自力で頑張って、スーパーレビュアーがモジュール間の問題に専念できるようにしてください。スーパーレビュアーと専門範囲を関連付けているのは、単に出発点を示すためだけです。どのスーパーレビュアーでもパッチの取り扱いができます。

スーパーレビュアーが問題を見逃したらどうなるのですか？

私たちはスーパーレビュアーが、全てを完璧に行うことを期待はしていません。見逃される問題もあることでしょう。スーパーレビュアーは日々新しいことを学んでいるでしょうが、学問に終点はありません。スーパーレビュアーがスーパーレビューを行うことによって、あなたの作業成果に関する責任が発生するわけではないのです。

スーパーレビューの前には必ず通常のレビューを受けなければなりませんか？

いいえ、逆の順序でも構いません。しかしながらスーパーレビュアーは、まず通常のレビューが終了してからスーパーレビューを始めたい、と決めることが出来ます。例えば、パッチの目的をどのようにして実現するかで議論が分かれており、パッチが完成するまでに大幅な変更があるかもしれないような場合です。

レビューやスーパーレビューの結果はどのように伝えられますか？

パッチがレビューやスーパーレビューを通った場合には、バグレポートのアタッチメントテーブルが "[name]:review+" や、 "[name]:super-review+" の状態になります。レビューを通らなかった場合には、"[name]:review-"、 "[name]:super-review-" となります。レビュアーがレビューフラグを設定する場合には、レビューの内容に関するコメントがそのバグに追記されることが多いでしょう。

どれくらいの時間で返答を貰えますか？

あなたが特定のレビュアーにスーパーレビューを依頼した場合、24 時間以内になんらかの返答をもらえるでしょう。最低限、次のような事項に関する返答があるはずです。(a) スーパーレビュアーがレビューを行えるかどうか、(b) もし可能な場合、完了までの時間。 スーパーレビュアーが要求どおりスーパーレビューを行うことが出来ないというだけの返答をもらうのに 24 時間も待つのは長すぎるということはわかっていますので、要求されたスーパーレビューを行えない場合には、できるだけ早く返事を出すよう、スーパーレビュアーにお願いしています。

多くのパッチには、有意義なコメントが 24 時間以内に付くでしょう。しかし、スーパーレビュアーには、24 時間以内にレビューを行う義務はありません。 スーパーレビュアーが後になってスーパーレビューを行えそうにない、あるいは時間的見通しを誤ったのに気付いた場合には、迅速にスーパーレビュアーからその旨を連絡されるでしょう。

自分のパッチが単純かつ小さなもので、スーパーレビューは必要無いと確信できる場合は、その旨のリクエスト ("requesting 'rs= '") を行ってください。スーパーレビュアーはパッチに「ゴム印」を付ける決定権を持っています。つまり、スーパーレビュアーはパッチが非常に基本的なものであり、スーパーレビューを飛ばしても構わないと合意したことになります。この場合、「ゴム印」の付いたパッチはすみやかに受理されるでしょう。

返答を貰えない場合にはどうすれば良いですか？

スーパーレビュアーが度忘れをしてパッチを受け付けない、ということもありえます。彼らは完璧ではなく、往々にしてオーバーワークです。24 時間以内に返答がない場合には再度 Ping (問い合わせ) を行ってください。または他のスーパーレビュアーをあたってください。この問題が頻繁に起こるのであれば、具体的なデータを mozilla.org のスタッフに知らせてください。(データ抜きで苦情をよこさないでください。「これに一週間かかりました」だけで、具体的なデータの量が充分でなければ、問題の原因を探り出すことはできません。) 私たちはトラッキングシステムを使用しているのですから、より良い解決策を見つけることができるでしょう。

24 時間以内にスーパーレビューを受けられる見込みはありますか？

それは、あなたのパッチと、スーパーレビュアーの作業負荷によります。各スーパーレビュアーの作業のやり方は人によって少しずつ異なっているので、スーパーレビュアーがコーディング、スーパーレビュー、そしてその他の作業に対してどのような優先順位を付けるのか、そしてあなたのパッチがそのどこに該当するか、を一つの型にはめることはできません。仕事が中断されることに慣れていて、簡単なレビューのためならすぐに本業を中断できるスーパーレビュアーもいるでしょう。別のスーパーレビュアーは、まず本業を完了させてからスーパーレビューにかかるでしょう。簡単にレビューでき、かつ重大問題を解決するようなパッチが関心を引きやすいということは確かです。また、数時間の濃密なレビューを必要とするような大きなパッチには時間がかかるということも明らかでしょう。

スーパーレビューを後回しにして、チェックインを先に行うことが出来ないのは何故ですか？

それには多くの理由があります。ここに、順不同でいくつかを挙げます。

スーパーレビュアーが探し出すのは、チェックインされた後に広範囲の影響を及ぼすような問題だからです。スーパーレビューで発見できるような問題でトランク上の全員が苦労するのでは意味がありません。ミスター・スポックなら「多数の善は、少数の善に勝る」と言うでしょう。

コードをチェックインすると、「パッチの作業はこれで終わりだ。次の作業に進もう」という心理学的な効果をもたらすようです。そうすると、パッチ変更の要求は、新たな別の作業と思われがちです。実際にはまだ完了していないのに、完了したような気分にさせるのは、馬鹿げたことです。

コードのチェックインの後にスーパーレビュアーがコメントを追加すると、そのコメントが実装されたかされていないかを追跡するのが困難になります。

スーパーレビュアーから、どのような回答を貰えるのですか？

典型的な返答としては、以下の様なものがあります：

• "sr=[name]." という承認。
• "r/sr=[name]." という承認。これはスーパーレビュアーが、パッチの関係する領域でレビュアーでもある場合に有り得ます。この返答の意味は、承認がレビューとスーパーレビューとのどちらとしても適用できる (両方ではない) ことを意味します。従って、パッチの作者にとって、第二のレビューを受けることが可能な人の幅が広がります。
• 変更の要求と変更されたパッチの再提出の要求。
• 変更の要求と、"once changes made, sr=[name]." (変更後は[name]にスーパーレビューを受けること) の様な但し書き。
• "rs=[name]." という承認。これは、パッチが小さいか自明のものであって、スーパーレビューは不要である、とスーパーレビュアーが判断したことを意味します。
• モジュール所有者に対して、まずレビューを完了するよう依頼。
• まずパッチを適用して、テストしてくれるよう誰かに依頼。(往々にして良い学習材料です)
• 他のスーパーレビュアーにパッチを取り扱ってくれるように依頼。
• 説明またはリスク分析の依頼。

スーパーレビュアーは、上記のリストに記載されていない回答をすることもあります。上記は全ての回答を網羅したものではありませんので、スーパーレビュアーがこのリストに縛られるのは私たちの望むところではありません。

なぜスーパーレビュアーから "rs=" を貰うまで待たなければならないのですか？ なぜ小さな変更であっても、スーパーレビューの必要が無いと勝手に判断してはならないのですか？

一貫性を保つには、チェックインアクセスが可能な全員よりも、25 人のグループのほうが簡単だからです。あなたのパッチが "rs" の範疇に属すると考えるのなら、リクエストにその旨を注記してください。これによって、レビューに割ける時間が少ないスーパーレビュアーに対して、そのパッチを見てみようという気にさせることができるかもしれません。もちろん、スーパーレビュアーは、リクエストを受け入れず、パッチにスーパーレビューが必要であると決めることが出来ます。もしあなたが全てのパッチに "rs wanted" 通知をつけたとするならば、「オオカミが来たとほらを吹く少年」だと見なされるという逆効果になり、"rs wanted" を付けても重視されなくなるでしょう。

スーパーレビュアーがモジュール所有者の場合、どうして R と SR の両方を与えることが出来ないのですか？

ソースコードをツリーに入れるにあたって、第三者の確認が必要と考えるからです。

私のパッチのプライオリティが低くて、当分の間レビューを受けられない、とレビュアーに言われた場合、どうしたら良いですか？

間違ってはいないのかもしれませんが、そのような返答を貰うのはいやですよね、別のスーパーレビュアーをあたってください。

スーパーレビュアーは、該当バグで明確に特定されていないような作業をパッチ作成者に依頼することが出来ますか？

はい、これに関して、スーパーレビュアーには自由裁量権があります。コードの品質の継続的な向上が私たちの望みであり、そのパッチに関連する追加的な変更を作成するのは正当なことでしょう。しかしながら、スーパーレビュアーは白紙委任状を持っているわけではありません。例えば、ミスタイプを修正するために、そのモジュール全体の文字列の実装を更新するような要求は、やり過ぎでしょう。特定のバグが関与する範囲と、スーパーレビュアーによって要求される変更の範囲とには、合理的な関係があるべきです。

スーパーレビュアーから、上記の文字列の例のような担当範囲外の変更を依頼されたら、どうすれば良いですか？

スーパーレビュアーに、その要求を新たなバグとして登録し、別のバグとして評価し優先順位付けし、修正することが可能であるか、尋ねてください。そして、この新しいバグが修正される前に、あなたが行った修正をチェックできないか、ということも確認してください。もしスーパーレビュアーと合意が出来なかった場合、他のスーパーレビュアー、または [email protected]、あるいはこの両方に、仲裁を頼んでください。見解の相違があるということを明記するのを忘れないようにしてください。他のレビュアーに第二のスーパーレビューを求め、より口当たりの良い結果を期待することは、更なる問題に繋がるでしょう。

スーパーレビュアーはユーザーインターフェースとユーザーエクスペリエンスの問題も担当しますか？

はい。現在私たちは、コードのチェックインに先立って、独立した「UI/UE レビュー」を行っていません。過去に何度か提案されたことはありますが、私たちは別のレベルや別のレビューを設けるつもりはありません。その代わりに、スーパーレビュアーに UI の問題も考慮するように頼んでいます。彼らは、(a) インターフェースの破壊、(b) 他の UI 要素や仕様との不一致、(c) 仕様にはない大幅な UI の変更、 (d) その他奇妙なインターフェース、に注目するでしょう。

(a) の場合、スーパーレビュアーは、スーパーレビューの一環として修正できるか尋ねるでしょう。 (b), (c), (d) に関しては、スーパーレビュアーが UI 開発者に声をかけるでしょう。スーパーレビュアーが個人的な好みで UI に関する決定を行うようなことはあってはならないと考えています。

理解できない変更や、どのように実装したらよいかわからない変更をスーパーレビュアーから依頼された場合、どうしたら良いですか？

スーパーレビュアーには要求仕様を明確にする責任がありますが、あなたのコードの修正方法を教える責任はありません。スーパーレビュアーの要求を実装するために何らかの勉強が必要な場合、コミュニティの知り合いや会社の同僚に聞く、IRC やニュースグループに質問する、などの方法があります。もし Mozilla の作業を行うために雇われているのなら、あなたの会社に学習システムや指導教育システムがあるかもしれません。

スーパーレビューにはトレーニング機構としての意図はありますか？

Yes でもあり、No でもあります。スーパーレビューは中上級者には優れた訓練機構です。一方で、経験が不十分な人にとって、スーパーレビューは恐ろしい訓練機構でもあります。コミュニティーの全体にわたって、基礎的な訓練を徹底的に広める必要がありますが、スーパーレビュアーの数はあまりにも少なく、また忙しすぎるので、マンツーマンで訓練を行う時間はほんの短い時間であっても捻出できません。そんな要求をすれば、燃え尽き症候群へのもうひとつの確実な道となるでしょう。私たちは、スーパーレビューが訓練の最終ステップとなるような、別の訓練方法を探しています。

どうしたらスーパーレビュアーになれますか？

まず、自分の胸に手を当てて考えてください。本当に、インフラのレビュアーになりたいですか？「スーパー」が意味する、製品のコア・シェルよりも魅惑的な何かに惑わされていませんか？それでもやはりスーパーレビュアーになりたいのなら、スーパーレビューの分野におけるあなたの才能を実証する方法を探してください。あなたが既にコードレビューに関っているのなら、範囲を拡大してスーパーレビューの対象も含めるようにしてください。また、スーパーレビュアーの中にはアシスタントがいればいいと思っている人がいるかもしれません。

スーパーレビュアーに次のようなことをアピールしてみましょう。

• 製品の基盤と統合の問題について理解していることを示すコードを書く。
• より良いレビュー、慎重なレビューを行う。
• コードレビューにおいて製品基盤と統合の問題を識別する。
• 明確なコメントでフィードバックを送り、製品基盤と統合の問題に関する教育技術を持っていることを示す。
• 良好なコミュニケーションが取れることを示す。
• Mozilla コミュニティへの尊敬の念を持っていることを示す。

自分の準備が整ったと思ったら、スーパーレビュアーグループにあなたの参加を提案してくれるスーパーレビュアーを探してください。スーパーレビュアーは、あなたの準備が整った、あるいは検討に値する程度に準備が整っていると考えれば、参加を提案してくれるでしょう。 明らかに力不足の人をスーパーレビュアーが推薦することは、まず無いでしょう。

スーパーレビュアーたちの間で、あなたには参加する資格があるという合意が出来たなら、あなたはグループの一員になれます。「合意」についての公式な定義はまだありませんし、明文化のためのプロセスもありません。当面はケースバイケースで対応したいと思います。必要となる前に、ではなく、必要に応じてプロセスを加えましょう。