Keyboard shortcuts

Press ← or → to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 15016] ファイルの概要

このコミットは、Go言語プロジェクトのコードレビューシステムにおける表示改善に関するものです。具体的には、コードレビューツールが生成する出力において、「Looks Good To Me (LGTM)」だけでなく、承認されていない（「not LGTM」または不承認）コメントも表示するように変更されています。これにより、レビューの状況がより包括的に把握できるようになります。

コミット

commit e1448c07e1365998b7e80e01d9f94c4e4345f7d5
Author: Russ Cox <rsc@golang.org>
Date:   Tue Jan 29 09:32:49 2013 -0800

    codereview: show 'not lgtms' in hg p output (with lgtms)
    
    R=golang-dev, iant
    CC=golang-dev
    https://golang.org/cl/7245043
---
 lib/codereview/codereview.py | 2 +-|
 1 file changed, 1 insertion(+), 1 deletion(-)|

diff --git a/lib/codereview/codereview.py b/lib/codereview/codereview.py
index 3d7b9ad5b1..0b7b5008ec 100644
--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -544,7 +544,7 @@ def LoadCL(ui, repo, name, web=True):
 		cl.private = d.get('private', False) != False
 		cl.lgtm = []
 		for m in d.get('messages', []):
-			if m.get('approval', False) == True:
+			if m.get('approval', False) == True or m.get('disapproval', False) == True:
 				who = re.sub('@.*', '', m.get('sender', ''))
 				text = re.sub("\n(.|\n)*", '', m.get('text', ''))
 				cl.lgtm.append((who, text))

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/e1448c07e1365998b7e80e01d9f94c4e4345f7d5

元コミット内容

codereview: show 'not lgtms' in hg p output (with lgtms)

このコミットは、コードレビューツールが生成する hg p コマンドの出力において、承認（LGTM）だけでなく、不承認（not LGTM）のコメントも表示するように変更するものです。

変更の背景

Go言語プロジェクトでは、Gerritというコードレビューシステムが広く利用されています。Gerritでは、提案された変更（パッチセット）に対してレビュー担当者がコメントを付け、承認（LGTM: Looks Good To Me）または不承認（-1, -2などのスコア、または明示的な不承認）の評価を行います。

このコミットが作成された背景には、おそらく、既存のコードレビューツール（lib/codereview/codereview.py）が、レビューの承認状況を把握する際に、LGTMのコメントのみを考慮し、不承認のコメントを見落としていたという問題があったと考えられます。開発者やレビュー担当者が hg p コマンド（Mercurialのパッチ関連の出力、またはGoプロジェクト内で使用されるカスタムコマンド/エイリアス）を使用してレビューの状況を確認する際、不承認のコメントが表示されないと、レビューが停滞している理由や、修正が必要な箇所が不明瞭になる可能性があります。

この変更は、レビュープロセスの透明性を高め、開発者がより迅速にフィードバックに対応できるようにすることを目的としています。不承認のコメントも出力に含めることで、レビューの全体像を把握しやすくなり、コードの品質向上に貢献します。

前提知識の解説

1. コードレビューシステム (Gerrit)

Go言語プロジェクトでは、Gerrit Code Reviewというオープンソースのウェブベースのコードレビューシステムが利用されています。GerritはGitの上に構築されており、以下のような特徴があります。

  • パッチベースのレビュー: 開発者は直接リポジトリにプッシュするのではなく、変更をGerritにパッチセットとしてアップロードします。これにより、レビュー担当者は変更を詳細に確認し、コメントや評価を付けることができます。
  • ウェブベースのUI: Gerritは、コードの差分表示、コメントの追加、レビューの評価（スコア付け）などを行うための直感的なウェブインターフェースを提供します。
  • レビューラベル: レビュー担当者は、変更に対して「Code-Review+1」（承認）、「Code-Review+2」（承認、マージ可能）、「Code-Review-1」（軽微な問題）、「Code-Review-2」（重大な問題、不承認）などのラベルを付与します。特に「Code-Review+2」は、その変更がマージ可能であることを示します。
  • LGTM (Looks Good To Me): コードレビューにおいて、変更が承認されたことを示す一般的な略語です。Gerritでは、レビュー担当者が承認のコメントやスコアを付けることでLGTMと見なされます。
  • TryBots: Gerritは、変更がマージされる前に自動テストを実行する「TryBots」と統合されています。これにより、コードの品質が保証され、リグレッションが防止されます。

2. Mercurial (hg) と hg p コマンド

Mercurial (hg) は、Gitと同様の分散型バージョン管理システムです。Go言語プロジェクトでは、かつてMercurialが主要なバージョン管理システムとして使用されていました（現在はGitに移行しています）。

コミットメッセージにある hg p output の hg p は、標準のMercurialコマンドではありません。これは、Goプロジェクトの内部ツールやスクリプトで定義されたカスタムエイリアス、または特定のパッチ管理に関連するコマンドである可能性が高いです。Mercurialでは、ユーザーが ~/.hgrc やリポジトリ内の .hg/hgrc ファイルにエイリアスを定義できます。この文脈では、おそらくコードレビューシステムと連携してパッチの情報を表示するためのコマンドとして機能していたと考えられます。

3. Pythonスクリプト (codereview.py)

lib/codereview/codereview.py は、Go言語プロジェクトのコードレビュープロセスを支援するためのPythonスクリプトです。このスクリプトは、Gerritなどのコードレビューシステムから情報を取得し、それを処理して、開発者にとって有用な形式で表示する役割を担っています。このコミットでは、特にレビューメッセージの処理方法が変更されています。

技術的詳細

このコミットの技術的な核心は、lib/codereview/codereview.py スクリプト内の LoadCL 関数におけるレビューメッセージの処理ロジックの変更です。

LoadCL 関数は、コードレビューシステムから変更リスト（Change List, CL）のデータをロードする役割を担っています。このデータには、レビューコメントや承認状況などのメッセージが含まれています。

変更前は、cl.lgtm というリストに、レビューメッセージの中から approval フラグが True に設定されているもの、つまり明示的に承認されたメッセージのみを追加していました。

if m.get('approval', False) == True:
    # ... LGTMメッセージをcl.lgtmに追加

このロジックでは、レビュー担当者が不承認のコメント（例: disapproval フラグが True のメッセージ）を送信した場合、その情報は cl.lgtm リストには含まれませんでした。結果として、hg p コマンドの出力など、この cl.lgtm リストを利用する箇所では、不承認の状況が反映されず、レビューの全体像を把握することが困難でした。

今回の変更では、この条件式に or m.get('disapproval', False) == True が追加されました。

if m.get('approval', False) == True or m.get('disapproval', False) == True:
    # ... LGTMまたは不承認メッセージをcl.lgtmに追加

これにより、cl.lgtm リストには、承認されたメッセージだけでなく、不承認のメッセージも含まれるようになります。cl.lgtm という変数名が「Looks Good To Me」を意味するにもかかわらず、不承認のメッセージも含むようになるのは、このリストが単なる承認の記録ではなく、レビューの評価（承認または不承認）が行われたメッセージ全般を収集するためのものとして再定義されたことを示唆しています。

この変更の目的は、hg p コマンドの出力に、承認と不承認の両方のレビューコメントを表示することで、レビューのステータスをより正確かつ包括的に反映させることです。これにより、開発者は、どのレビューが承認され、どのレビューがまだ問題点を抱えているのかを、一目で把握できるようになります。

コアとなるコードの変更箇所

変更は lib/codereview/codereview.py ファイルの1箇所のみです。

--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -544,7 +544,7 @@ def LoadCL(ui, repo, name, web=True):
 		cl.private = d.get('private', False) != False
 		cl.lgtm = []
 		for m in d.get('messages', []):
-			if m.get('approval', False) == True:
+			if m.get('approval', False) == True or m.get('disapproval', False) == True:
 				who = re.sub('@.*', '', m.get('sender', ''))
 				text = re.sub("\n(.|\n)*", '', m.get('text', ''))
 				cl.lgtm.append((who, text))

具体的には、545行目の if 文の条件式が変更されています。

コアとなるコードの解説

変更された行は、コードレビューメッセージを処理し、cl.lgtm リストに含めるかどうかを決定する条件式です。

  • 変更前: if m.get('approval', False) == True:
    • これは、メッセージ m が approval というキーを持ち、その値が True である場合にのみ、そのメッセージを cl.lgtm リストに追加することを意味していました。つまり、明示的な承認メッセージのみが収集されていました。
  • 変更後: if m.get('approval', False) == True or m.get('disapproval', False) == True:
    • この変更により、メッセージ m が approval が True であるか、または disapproval が True である場合に、そのメッセージが cl.lgtm リストに追加されるようになりました。
    • m.get('approval', False) は、メッセージ辞書 m から approval キーの値を取得します。もしキーが存在しない場合はデフォルト値として False を返します。
    • m.get('disapproval', False) も同様に、disapproval キーの値を取得し、存在しない場合は False を返します。
    • or 演算子により、どちらか一方でも条件が満たされれば（つまり、承認または不承認のいずれかのフラグが True であれば）、そのメッセージが cl.lgtm リストに追加されます。

この変更の意図は、cl.lgtm リストが、単に「承認された」メッセージだけでなく、「レビューによって評価された（承認または不承認）」メッセージ全般を保持するようにすることです。これにより、このリストを利用するダウンストリームの処理（例: hg p コマンドの出力生成）において、レビューの全体的なステータス（承認と不承認の両方）が正確に反映されるようになります。

関連リンク

参考にした情報源リンク