[インデックス 13002] ファイルの概要
このコミットは、Go言語プロジェクトのコードレビューダッシュボードに関連する変更です。具体的には、コードレビューシステムから取得した「LGTM (Looks Good To Me)」の承認情報を処理する際に、重複するLGTMを排除するための修正が行われています。これにより、ダッシュボードに表示されるLGTMのリストが正確になり、同じユーザーからの複数の承認が重複してカウントされることがなくなります。
コミット
- コミットハッシュ:
83aa040c450a6f1af22a61c2691e42e481b87beb
- 作者: David Symonds dsymonds@golang.org
- コミット日時: 2012年5月1日 火曜日 11:41:32 +1000
- 変更ファイル:
misc/dashboard/codereview/dashboard/cl.go
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/83aa040c450a6f1af22a61c2691e42e481b87beb
元コミット内容
misc/dashboard/codereview: de-dup LGTMs.
R=golang-dev, r
CC=golang-dev
https://golang.org/cl/6127066
変更の背景
この変更の背景には、Go言語プロジェクトが当時利用していたコードレビューシステム(Rietveld/Gerritベース)の特性と、それを可視化するダッシュボードの要件があります。
以前の実装では、コードレビューのメッセージを処理する際に、あるユーザーが複数回「LGTM」を送信した場合や、何らかの理由で同じ承認メッセージが複数回処理された場合に、ダッシュボードに表示されるLGTMのリスト(cl.LGTMs
)に同じユーザー名が重複して追加されてしまう問題がありました。これは、LGTMの数を正確に把握したり、承認したユニークなユーザーを識別したりする上で不都合でした。
コミットメッセージにある// TODO(dsymonds): De-dupe LGTMs.
というコメントは、この重複排除の必要性が以前から認識されており、将来的な改善点として残されていたことを示しています。今回のコミットは、このTODO項目に対応し、LGTMのリストから重複を排除することで、ダッシュボードの表示をより正確にするために行われました。
前提知識の解説
このコミットを理解するためには、以下の技術的・概念的な知識が必要です。
-
LGTM (Looks Good To Me):
- ソフトウェア開発におけるコードレビュープロセスで使われる略語で、「このコードは問題ない、承認する」という意味合いを持ちます。
- 多くのコードレビューシステム(例: Gerrit, GitHub Pull Requests, GitLab Merge Requestsなど)では、レビュアーがコードの品質や機能性を確認し、問題がなければLGTMを付与することで承認の意思を示します。
- プロジェクトによっては、特定の数のLGTMが集まるとコードがマージ可能になる、といったルールが設けられています。
-
Go言語の
map
(マップ):- Go言語における組み込みのデータ構造の一つで、キーと値のペアを格納するコレクションです。他の言語では「ハッシュマップ」「辞書」「連想配列」などと呼ばれることもあります。
- マップのキーは一意であるという特性があります。つまり、同じキーを複数回追加しようとしても、既存の値が上書きされるだけで、新しいエントリが追加されることはありません。この特性は、重複する要素を排除してユニークな要素のセットを作成する際に非常に有用です。
- 構文例:
m := make(map[string]int)
は文字列をキー、整数を値とするマップを作成します。m["key"] = value
で要素を追加・更新します。
-
Go言語の
slice
(スライス):- Go言語における可変長シーケンス(配列のようなもの)です。内部的には固定長の配列を参照していますが、スライス自体は動的にサイズを変更できます。
append
関数を使って要素を追加できます。slice = append(slice, element)
のように使用し、必要に応じて新しい基底配列が割り当てられることがあります。
-
コードレビューシステム (Rietveld/Gerrit):
- Go言語プロジェクトは、初期にはGoogleが開発したRietveldというコードレビューツールを、その後Gerritというツールを利用していました。これらのシステムは、開発者が提出した変更(チェンジリスト、またはプルリクエスト)に対して、他の開発者がコメントを付けたり、承認(LGTM)を与えたりする機能を提供します。
- これらのシステムは、APIを通じてレビューコメントや承認情報を取得できるため、本コミットで修正されているようなダッシュボードアプリケーションがそれらの情報を集約・表示することが可能になります。
-
Google App Engine (App Engine):
- Googleが提供するPaaS (Platform as a Service) で、ウェブアプリケーションやモバイルバックエンドを構築・デプロイするためのプラットフォームです。
- このコミットで変更されている
cl.go
ファイル内のfunc updateCL(c appengine.Context, n string) error
という関数シグネチャから、このダッシュボードアプリケーションがGoogle App Engine上で動作していることがわかります。appengine.Context
はApp Engineのサービス(データストア、Memcacheなど)にアクセスするためのコンテキストを提供します。
技術的詳細
このコミットの主要な目的は、コードレビューの承認者リスト(LGTMs)から重複を排除することです。以前の実装では、apiResp.Messages
(コードレビューシステムからのメッセージリスト)をループ処理し、各メッセージが承認(msg.Approval
がtrue
)である場合に、そのメッセージの送信者(s
)を直接cl.LGTMs
というスライスに追加していました。この方法では、同じ送信者が複数回承認した場合、その名前がcl.LGTMs
に複数回現れてしまいます。
この問題を解決するために、以下の変更が導入されました。
-
lgtm
マップの導入:lgtm := make(map[string]bool)
という新しいマップが導入されました。このマップは、LGTMを付与したユニークなユーザー名をキーとして保持するために使用されます。値はbool
型ですが、ここでは単にキーの存在を確認するために使われるため、true
で固定されます。
-
LGTMの仮格納:
- メッセージをループ処理する中で、
msg.Approval
がtrue
の場合、以前は直接cl.LGTMs
にs
をappend
していましたが、この変更ではlgtm[s] = true
として、lgtm
マップに送信者s
を追加するように変更されました。マップのキーは一意であるため、同じ送信者が複数回現れても、マップにはその送信者のエントリが一つだけ保持されます。
- メッセージをループ処理する中で、
-
重複排除後の
cl.LGTMs
への反映:- すべてのメッセージの処理が完了した後、
for l := range lgtm { cl.LGTMs = append(cl.LGTMs, l) }
という新しいループが追加されました。 - このループでは、
lgtm
マップのすべてのキー(つまり、重複が排除されたユニークなLGTM承認者)をイテレートし、それらを最終的なcl.LGTMs
スライスに追加します。これにより、cl.LGTMs
には重複のないLGTM承認者のリストが格納されることになります。
- すべてのメッセージの処理が完了した後、
このアプローチにより、コードレビューシステムから取得されるメッセージの順序や重複に関わらず、ダッシュボードには常にユニークなLGTM承認者のリストが表示されるようになります。
コアとなるコードの変更箇所
--- a/misc/dashboard/codereview/dashboard/cl.go
+++ b/misc/dashboard/codereview/dashboard/cl.go
@@ -304,6 +304,7 @@ func updateCL(c appengine.Context, n string) error {
if i := strings.Index(cl.FirstLine, "\n"); i >= 0 {
cl.FirstLine = cl.FirstLine[:i]
}
+ lgtm := make(map[string]bool)
rcpt := make(map[string]bool)
for _, msg := range apiResp.Messages {
s, rev := msg.Sender, false
@@ -320,14 +321,16 @@ func updateCL(c appengine.Context, n string) error {
}
if msg.Approval {
- // TODO(dsymonds): De-dupe LGTMs.
- cl.LGTMs = append(cl.LGTMs, s)
+ lgtm[s] = true
}
for _, r := range msg.Recipients {
rcpt[r] = true
}
}
+ for l := range lgtm {
+ cl.LGTMs = append(cl.LGTMs, l)
+ }
for r := range rcpt {
cl.Recipients = append(cl.Recipients, r)
}
コアとなるコードの解説
-
+ lgtm := make(map[string]bool)
(行307):updateCL
関数の冒頭で、lgtm
という名前の新しいマップが初期化されます。このマップは、LGTMを付与したユーザーのユニークな名前を一時的に保持するために使用されます。キーはユーザー名(string
)、値はbool
型ですが、ここでは値自体は重要ではなく、キーの一意性を利用します。
-
- // TODO(dsymonds): De-dupe LGTMs.
(行321):- 以前のコードにあった、LGTMの重複排除に関するTODOコメントが削除されました。これは、このコミットによってそのTODOが完了したことを示しています。
-
- cl.LGTMs = append(cl.LGTMs, s)
(行322):- LGTM承認があった場合に、直接
cl.LGTMs
スライスに送信者s
を追加していた行が削除されました。この直接追加が重複の原因でした。
- LGTM承認があった場合に、直接
-
+ lgtm[s] = true
(行322):- 上記の削除された行の代わりに、LGTM承認があった場合に送信者
s
をlgtm
マップのキーとして追加する処理が挿入されました。マップの特性により、同じs
が複数回追加されても、マップ内にはそのs
に対応するエントリは一つしか存在しません。これにより、自動的に重複が排除されます。
- 上記の削除された行の代わりに、LGTM承認があった場合に送信者
-
+ for l := range lgtm {
(行328):- すべてのメッセージの処理が完了した後、新しく追加されたループです。このループは、
lgtm
マップのすべてのキー(つまり、ユニークなLGTM承認者)をイテレートします。
- すべてのメッセージの処理が完了した後、新しく追加されたループです。このループは、
-
+ cl.LGTMs = append(cl.LGTMs, l)
(行329):lgtm
マップから取得したユニークなユーザー名l
を、最終的にダッシュボードに表示されるcl.LGTMs
スライスに追加します。このステップにより、cl.LGTMs
には重複のないLGTM承認者のリストが格納されます。
この変更により、cl.LGTMs
スライスは、コードレビューシステムから取得されたメッセージの順序や重複に関わらず、常にユニークなLGTM承認者のリストを正確に反映するようになりました。
関連リンク
- Go言語のチェンジリスト:
https://golang.org/cl/6127066
- これは、Go言語プロジェクトが当時利用していたコードレビューシステム(RietveldまたはGerrit)におけるチェンジリスト(変更セット)へのリンクです。このリンクを辿ることで、このコミットが元々どのようなコードレビューを経て承認されたのか、関連するコメントや議論などを確認できます。Goプロジェクトでは、GitHubにマージされる前に、このような内部のコードレビューシステムで変更がレビューされるのが一般的でした。
参考にした情報源リンク
- Go言語の公式ドキュメント (map, slice, appendなど): https://go.dev/doc/
- Google App Engineのドキュメント (当時の情報): https://cloud.google.com/appengine/docs (現在のドキュメントは更新されていますが、当時の概念を理解するのに役立ちます)
- コードレビューの概念 (LGTMなど): 一般的なソフトウェア開発プラクティスに関する情報源
- Rietveld / Gerrit コードレビューシステム: https://code.google.com/p/rietveld/ (Rietveldは現在アーカイブされていますが、当時の情報源として)