Reviewing a Patch/ja

From PostgreSQL wiki

Jump to: navigation, search

PostgreSQL に対して投稿されたパッチに対して、多くの人々が、自身には完全なレビューをするだけの力が無いと感じています。しかし、もしあなたがまだコミッタで無いならば、それは技術的には当然なのです。プロジェクト全体もかなり大きくなっており、全貌を把握するのは難しくなっています。しかし、レビューにはたくさんの異なった仕事があり、もしあなたがそのすべてをできなかったにしtめお、早期段階の仕事に貢献することは十分に可能です。

もしあなたがパッチを適用し、新しい機能を試したのならば、それはすでにレビューをし始めていることになります。

最終的なコミッタは、コードベースに取り込む際に彼ら独自のレビューを行うでしょう。レビューアの仕事は、前もって問題を取り除くことで、コミッタの仕事を減らすことです。レビューアには完璧を求められることは無く、見つけられるだけの問題を見つけることが仕事になります。もしあなたが、そのパッチがコミッタの詳細レビューに足る品質になったと判断したら、そこでレビューは完了します。レビュー結果の報告の形式は、このパッチレビューを参考にしてください。もちろん、パッチの目的が異なれば、レビュー結果はかなり異なるものになるかもしれません。

もしあなたがこれから説明する仕事に取り組んでくれるのならば、とても助かります。どのような人材が求められているかは、Josh Tolley 氏のプレゼンテーションに、冗談めかしながらも、それほど不正確でない説明があります。

現在のコミットフェスタは ここで、あなたに手伝ってもらいたいことが多くあります。ラウンド・ロビン・レビューアへの参加は ここからできます。参加したならば、まずは自己紹介をして欲しいです。

一般的には、パッチをレビューしていく段階は、以下の順になります:

Contents

提案レビュー (必要スキル: パッチを交えた英語での議論)

  • パッチは コンテキストDIFF 形式か?
  • 現在の git master にエラー無く適用できるか?
  • テストやドキュメントを含んでいるか?

仕様レビュー (必要スキル: 仕様を読む)

そのパッチが何を実現しようとしているかを読み取り、以下を考察します:

  • そのパッチは実際に何をしようとしているのか?
  • そのパッチは本当に必要か?
  • 既存の機能との重複は無いか?
  • 標準SQLに従っているか? またはコミュニティの同意を得た動作か?
  • pg_dump のサポートがあるか?
  • 適用することで、何か危険性は無いか?
  • 基本がなっているか? (原文: Have all the bases been covered?)

機能テスト (必要スキル: パッチの適用, コンパイル, エラーログの確認)

パッチを適用し、コンパイルして、以下をテストします:

  • 公言通りの動作をするか?
  • パッチの作者が見逃している重箱の隅は無いか?
  • アサーションの失敗や、クラッシュは無いか?
    • レビューアは、configure する際に --enable-cassert--enable-debug オプションを有効にすべきです。方法は Working with CVS に載っています。これらを使うと、ミスを発見しやすくなります。しかし、これらのオプションを使うと PostgreSQL の性能が劣化します。もし、パッチが他の用途の性能を劣化させていないかなどの性能試験を行うならば、これらのオプションを off にすべきです。また、すべてではないものの、--enable-cassert の性能ペナルティのうちのいくらかは、postgresql.conf に debug_assertions = false を指定することで低減できます。詳細は Developer Options を参照してください。このオプションは --enable-cassert 付きでビルドするとデフォルトで有効 (true) になります。また、--enable-debug オプションは、パフォーマンスには影響が無いはずです。少なくとも gcc では確認されており、他のコンパイラでも同様である可能性が高いと考えられます。

性能試験 (必要スキル: 性能測定)

  • そのパッチが他のシンプルなテストの性能を劣化させないか?
  • 性能向上のt前のパッチであるならば、本当に性能は向上しているか?
  • そのパッチの影響で、他の機能の性能が劣化することはないか?

コーディング・レビュー (必要スキル: コーディング規約の確認, 移植性検討, 少々のCを読む技術)

そのコードが行った変更を詳細に読み解き、以下のことを確認します:

  • このプロジェクトの コーディング規約 に従っているか?
  • (他のプラットホームへの) 移植性に問題は無いか?
  • (Linux だけでなく) Windows や BSD でも動作するか?
  • コード中のコメントが十分かつ正確か?
  • コードの説明とその動作で、言行一致しているか?
  • コンパイルする際に警告を出力しないか?
  • クラッシュにつながる操作を許していないか?

アーキテクチャ・レビュー (必要スキル: PostgreSQL全体のアーキテクチャの理解)

そのコードが行った変更を、PostgreSQL全体の構成と照らし合わせて、以下を確認します:

  • 他の機能やモジュールと照らし合わせても、一貫性があるか?
  • 将来問題になり得る相互依存関係が無いか?

レビューのレビュー

  • そのレビューアは本来やるべきことをすべて行ったか?
Personal tools