プルリクエストのレビュー

プロジェクトには多くのプルリクエストがプッシュされており、それらをレビューしたりテストしたりすることでコアチームを支援することができます。

このWikiでは、この作業をどのように支援するかを説明したいと思います。

なぜこれが重要なのか?
2020年4月以降、ブルリクエストはプロジェクトへのコードの貢献のデフォルトの方法(Fork and Branch Git ワークフロー)となっています。ご存知のように、iDempiereはオープンソースのソフトウェアプロジェクトであり、コードはプロジェクトの最も重要な部分です。これらのバグ修正や機能追加は、コードが適切に書かれていれば、コミュニティの全員に利益をもたらします。

iDempiereのリーダーたちは、最高品質のコードを維持することを重視しています。彼らは、トランクに入るものや変更すべきものをレビューし、フィルタリングすることで、これを実現しています。問題は、プロジェクトが多くのリクエストを受ければ受けるほど、キューが長くなり、時間の経過とともに迷走するリクエストが出てくることです。

プルリクエストをレビューすることで、最初のフィルタになることができ、適切に実行すれば、リーダーの肩から多くの作業を省くことができ、重要なコードをより早くトランクに入れることができます。

プルリクエストをレビューするには?

コードがトランクに入る前にレビューしなければならないことはたくさんありますが、大まかには2つにまとめることができます。

1. コードはきちんと書かれていて、付随的な損害やセキュリティ侵害をもたらさないか？
2. コードは意図した通りの動作をしているか？

ローカル環境に何も足さなくてもできることは、まずコードを見直すことです。

コードレビュー

• オープンになっているプルリクエストから、１つを選んで下さい。
• プルリクエストに対して行われたコミットのリストを開くと、開発者によって行われたコードの変更を見ることができます。
• Check that the submitted code complies with the project guidelines to contribute to the trunk. The main points to consider are:
  • PostgreSQL と Oracle の両方で動作しなければなりません。
  • 可能な限りSQLコードの使用を避け、代わりにモデルクラスを使用してください。
  • データベースに変更があった場合は、移行スクリプトが存在します。
  • DBにSQLでアクセスする際には、すべてのカーソルは"finally"句で、閉じなければなりません。
  • コードはすべて英語で書かれています。
  • プルリクエストは目的(例えば、リファクタリングと新機能など)を混在させることなく、チケットの目的に対応するために必要なクラスのみを変更します。

コードがよく書かれていること、プロジェクトのガイドラインやベストプラクティスを満たしていることを確認したら、ローカル環境にコードを引っ張り出してテストします。

機能のレビュー

あなたのローカル環境で実行して下さい。:

git checkout master
git pull upstream master
git push
git pull --no-commit https://github.com/(pullrequest-github-user)/idempiere.git IDEMPIERE-(Jira-ticket-number) (for example: git pull --no-commit https://github.com/globalqss/idempiere.git IDEMPIERE-1848) 1

1 --no-commit を指定していても、git がコミットをログに追加してしまうことがあります。そのような場合は、git のログを確認して git reset --soft HEAD~ を実行すると、ログからコミットを削除してツリーをきれいに保つことができます。

一度、ローカル環境の変化を把握しておきましょう。

1. 必要に応じてマイグレーションスクリプトを適用して下さい。
2. iDempiereを起動してテストして下さい。
3. 可能な限りのシナリオをテストしてください。そのチケットで解決しようとしていたことをテストするだけでなく、以前の機能がコミットの影響を受けていないかどうかも確認してください。システムは、新たに追加された以外は以前と同じように動作するはずです。

マスターをクリーンに保つためにすべてを元に戻して下さい。

レビューを実行した後は、Jira チケットで発見したことを報告したり、プルリクエストに直接コメントを追加したりしてください。

テストした内容と結果をできるだけ具体的に記述して下さい。

何か問題点が見つかった場合は、何か提案があれば、それをどのように解決できると思うかコメントしてください。

結果が肯定的な場合は、チケットをテストしてみて、解決策が問題ないことがわかったとコメントしてください。

TL;DR;(Too Long, Didn't Read 長すぎると読まれません)

1. オープンなプルリクエストをひとつ選んで下さい。
2. Do a code review based on the project's guidelines to contribute to the trunk.
3. チケットの機能をテストして下さい。
4. チケットによる巻き添え被害やセキュリティ違反がないことを確認してください。
5. 発見したことを JIRA に報告するか、プルリクエストに直接報告してください。

思慮深くテストし、何をしたか、何をしなかったかについて完全に正直になることをお勧めします。これはオープンソースのコミュニティであり、あなたは多くの目で見られており、あなたの評判はあなたが何をするかによって利益を得たり傷ついたりします。