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

このコミットは、GoプロジェクトのコードレビューツールであるRietveldに関連するlib/codereview/codereview.pyファイルに対する変更です。このファイルは、Goプロジェクトのコードレビュープロセスを管理するためのスクリプトやユーティリティを含んでいます。具体的には、コミットメッセージの生成、レビュー担当者の管理、LGTM（Looks Good To Me）の記録など、コードレビューのワークフローを自動化・支援する機能を提供しています。

コミット

このコミットは、Goプロジェクトのコードレビューシステムにおいて、コミットメッセージにLGTM=行を追加する機能が導入されました。これにより、実際にコード変更を承認したレビュアーを明示的に記録できるようになります。

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

https://github.com/golang/go/commit/672ab62981190504b8524ee093dd67780e589473

元コミット内容

lib/codereview: add LGTM= line to commit messages

The R= is populated by Rietveld, so it's basically
anyone who replied to the CL. The LGTM= is meant
to record who actually signed off on the CL.

LGTM=r
R=r
CC=golang-codereviews
https://golang.org/cl/55390043

変更の背景

Goプロジェクトでは、Googleが開発したコードレビューツールであるRietveldをベースにしたシステムが使用されていました。従来のシステムでは、コミットメッセージにR=（Reviewer）という行が含まれていましたが、これはRietveld上でコメントを投稿したすべてのユーザーが自動的に含まれるものでした。しかし、これは必ずしもコード変更を正式に承認したレビュアーを正確に反映しているわけではありませんでした。

このコミットの背景には、以下の課題がありました。

1. 承認者の明確化の必要性: R=行だけでは、誰が最終的にコード変更を承認したのかが不明確でした。特に、複数のレビュアーが関与する場合、誰が「最終的な承認者」であるかを明確にする必要がありました。
2. コードレビュープロセスの改善: より厳格なコード品質管理と責任の所在を明確にするため、正式な承認プロセスをコミットメッセージに反映させることが求められました。
3. Rietveldの機能との連携: Rietveldには「LGTM」という承認機能がありましたが、その情報がコミットメッセージに直接反映されていませんでした。この変更により、Rietveldでの承認がコミットメッセージに自動的に記録されるようになります。

この変更により、LGTM=行がコミットメッセージに追加されることで、コード変更が誰によって正式に承認されたのかが一目でわかるようになり、コードレビュープロセスの透明性と信頼性が向上しました。

前提知識の解説

このコミットを理解するためには、以下の前提知識が必要です。

• コードレビュー (Code Review): ソフトウェア開発プロセスにおいて、他の開発者が書いたコードを別の開発者が確認し、品質、正確性、効率性、セキュリティなどの観点からフィードバックを提供するプロセスです。バグの早期発見、知識共有、コード品質の向上に寄与します。
• Rietveld: Googleが開発したオープンソースのコードレビューツールです。PerforceやGitなどのバージョン管理システムと連携し、変更セット（Change List, CL）に対するコメント、承認、差分表示などの機能を提供します。Goプロジェクトでは、このRietveldをベースにしたカスタムツールが使用されていました。
• Change List (CL): RietveldやGerritなどのコードレビューシステムにおける、一連のコード変更の単位です。通常、一つの機能追加やバグ修正に対応します。
• LGTM (Looks Good To Me): コードレビューにおいて、レビュアーが提案されたコード変更を承認し、マージしても良いと判断したことを示す略語です。Rietveldなどのツールでは、LGTMボタンやコメントで承認を示すことができます。
• コミットメッセージ (Commit Message): バージョン管理システム（Gitなど）において、コード変更をリポジトリに記録する際に付随させる説明文です。変更内容、目的、関連する課題などを記述し、後から変更履歴を追跡する際に重要な情報となります。
• R=行 (Reviewer Line): 従来のGoプロジェクトのコミットメッセージに含まれていた行で、コードレビューに参加したレビュアーを示すものでした。Rietveld上でCLにコメントを投稿したユーザーが自動的に追加される傾向がありました。
• LGTM=行 (Looks Good To Me Line): このコミットで導入された新しいコミットメッセージの行で、実際にコード変更を正式に承認したレビュアーを示すものです。

技術的詳細

このコミットは、lib/codereview/codereview.pyファイルに対して以下の主要な変更を加えています。

1. CL.lgtmの構造変更:

   • 変更前: cl.lgtmは(who, line)のタプルを格納していました。whoはレビュアー、lineはLGTMコメントのテキストです。
   • 変更後: cl.lgtmは(who, line, approval)の3要素タプルを格納するようになりました。approvalはブール値で、そのLGTMが正式な承認（True）であるか、単なるコメント（False）であるかを示します。これにより、Rietveldの承認ステータスをより正確に反映できるようになりました。

2. JoinComma関数の変更:

   • 変更前: 単純にリストの要素をカンマで結合するだけでした。
   • 変更後: JoinComma関数は、結合する前にリスト内の重複する要素を削除するロジックが追加されました。これは、複数のLGTMやレビューコメントがあった場合に、同じレビュアーが複数回表示されるのを防ぐためです。seen辞書とuniqリストを使用して、一意な要素のみを抽出しています。

3. LoadCL関数でのLGTM情報の取得:

   • LoadCL関数は、RietveldからCLの情報をロードする際に、各メッセージのapprovalフィールドをチェックするようになりました。
   • メッセージが承認（approval: True）または不承認（disapproval: True）である場合、その情報をcl.lgtmリストに追加する際に、3番目の要素としてm.get('approval', False)（承認ステータス）を渡すようになりました。

4. submit関数でのコミットメッセージ生成ロジックの変更:

   • submit関数は、CLをリポジトリにコミットする際にコミットメッセージを生成します。
   • LGTMチェックの追加: if not cl.lgtm and not opts.get('tbr'): raise hg_util.Abort("this CL has not been LGTM'ed")という行が追加されました。これは、CLがLGTMされていない場合（または--tbrオプションが指定されていない場合）、コミットを中止する強制的なチェックです。これにより、LGTMなしでのコミットが防止されます。
   • LGTM=行の追加: if cl.lgtm: about += "LGTM=" + JoinComma([CutDomain(who) for (who, line, approval) in cl.lgtm if approval]) + "\\n"という行が追加されました。これにより、cl.lgtmリストから正式に承認したレビュアー（approvalがTrueのLGTM）を抽出し、LGTM=行としてコミットメッセージに追加します。
   • R=行の生成順序の変更: R=行の生成がLGTM=行の後に移動しました。これにより、LGTM=がより重要な情報として先に表示されるようになります。また、reviewer変数を導入し、tbr（To Be Reviewed）オプションが指定された場合のレビュアーリストの処理を改善しています。

これらの変更により、Goのコードレビュープロセスはより厳格になり、誰がコード変更を承認したのかがコミット履歴に明確に記録されるようになりました。

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

diff --git a/lib/codereview/codereview.py b/lib/codereview/codereview.py
index ec3e9c199d..2618ef9301 100644
--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -277,7 +277,7 @@ class CL(object):
 			s += "\tAuthor: " + cl.copied_from + "\n"
 		if not quick:
 			s += "\tReviewer: " + JoinComma(cl.reviewer) + "\n"
-			for (who, line) in cl.lgtm:
+			for (who, line, _) in cl.lgtm:
 				s += "\t\t" + who + ": " + line + "\n"
 			s += "\tCC: " + JoinComma(cl.cc) + "\n"
 		s += "\tFiles:\n"
@@ -493,9 +493,15 @@ def CutDomain(s):
 	return s
 
 def JoinComma(l):
+	seen = {}
+	uniq = []
 	for s in l:
 		typecheck(s, str)
-	return ", ".join(l)
+		if s not in seen:
+			seen[s] = True
+			uniq.append(s)
+			
+	return ", ".join(uniq)
 
 def ExceptionDetail():
 	s = str(sys.exc_info()[0])
@@ -556,7 +556,7 @@ def LoadCL(ui, repo, name, web=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))
+				cl.lgtm.append((who, text, m.get('approval', False)))
 
 	set_status("loaded CL " + name)
 	return cl, ''
@@ -1928,12 +1934,21 @@ def submit(ui, repo, *pats, **opts):
 	typecheck(userline, str)
 
 	about = ""
-	if cl.reviewer:
-		about += "R=" + JoinComma([CutDomain(s) for s in cl.reviewer]) + "\n"
+
+	if not cl.lgtm and not opts.get('tbr'):
+		raise hg_util.Abort("this CL has not been LGTM'ed")
+	if cl.lgtm:
+		about += "LGTM=" + JoinComma([CutDomain(who) for (who, line, approval) in cl.lgtm if approval]) + "\n"
+	reviewer = cl.reviewer
 	if opts.get('tbr'):
 		tbr = SplitCommaSpace(opts.get('tbr'))
+		for name in tbr:
+			if name.startswith('golang-'):
+				raise hg_util.Abort("--tbr requires a person, not a mailing list")
 		cl.reviewer = Add(cl.reviewer, tbr)
 		about += "TBR=" + JoinComma([CutDomain(s) for s in tbr]) + "\n"
+	if reviewer:
+		about += "R=" + JoinComma([CutDomain(s) for s in reviewer]) + "\n"
 	if cl.cc:
 		about += "CC=" + JoinComma([CutDomain(s) for s in cl.cc]) + "\n"
 

コアとなるコードの解説

• lib/codereview/codereview.pyの変更点:
  • L280: for (who, line) in cl.lgtm: が for (who, line, _) in cl.lgtm: に変更されました。これは、cl.lgtmの要素が2つ組から3つ組（レビュアー、コメント、承認ステータス）になったことを反映しています。_は承認ステータスをここでは使用しないことを示します。
  • L496-L502 (JoinComma関数): JoinComma関数に重複排除ロジックが追加されました。seen辞書とuniqリストを使用して、入力リストlから重複する要素を取り除き、一意な要素のみをカンマ区切りで結合して返します。これにより、レビュアー名などが重複して表示されるのを防ぎます。
  • L559: cl.lgtm.append((who, text)) が cl.lgtm.append((who, text, m.get('approval', False))) に変更されました。Rietveldから取得したメッセージのapprovalフィールド（承認ステータス）をcl.lgtmリストの3番目の要素として格納するようになりました。
  • L1931-L1933: if not cl.lgtm and not opts.get('tbr'): raise hg_util.Abort("this CL has not been LGTM'ed") が追加されました。これは、CLがLGTMされていない、かつ--tbrオプションが指定されていない場合にコミットを中止する強制的なチェックです。
  • L1934-L1935: if cl.lgtm: about += "LGTM=" + JoinComma([CutDomain(who) for (who, line, approval) in cl.lgtm if approval]) + "\n" が追加されました。これは、cl.lgtmリストから正式に承認されたレビュアー（approvalがTrueのLGTM）を抽出し、LGTM=行としてコミットメッセージに追加する部分です。
  • L1936: reviewer = cl.reviewer が追加され、元のレビュアーリストを保持します。
  • L1937-L1941: tbrオプションの処理が変更され、golang-で始まるメーリングリスト名を--tbrに指定できないようにするチェックが追加されました。
  • L1942-L1943: if reviewer: about += "R=" + JoinComma([CutDomain(s) for s in reviewer]) + "\n" が追加されました。R=行の生成がLGTM=行の後に移動し、元のレビュアーリスト（reviewer変数）を使用するように変更されました。

これらの変更により、Goのコードレビュープロセスはより堅牢になり、コミットメッセージにLGTM情報が明確に記録されるようになりました。

関連リンク

• Go CL 55390043: https://golang.org/cl/55390043

参考にした情報源リンク

• Rietveld Code Review Tool: https://code.google.com/p/rietveld/ (現在はアーカイブされていますが、Rietveldに関する一般的な情報源として)
• Go Contribution Guidelines (一般的なGoのコードレビュープロセスについて): https://go.dev/doc/contribute
• LGTM (Looks Good To Me) の意味: https://en.wikipedia.org/wiki/LGTM
• Pythonのリスト内包表記とタプルアンパックに関する情報 (コード理解のため): https://docs.python.org/3/tutorial/datastructures.html
• 正規表現 (reモジュール) に関する情報 (コード理解のため): https://docs.python.org/3/library/re.html
• hg_util.Abort (Mercurialユーティリティに関する情報): GoプロジェクトがMercurialを使用していた時期のユーティリティ関数。