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

[インデックス 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のリストから重複を排除することで、ダッシュボードの表示をより正確にするために行われました。

前提知識の解説

このコミットを理解するためには、以下の技術的・概念的な知識が必要です。

  1. LGTM (Looks Good To Me):

    • ソフトウェア開発におけるコードレビュープロセスで使われる略語で、「このコードは問題ない、承認する」という意味合いを持ちます。
    • 多くのコードレビューシステム(例: Gerrit, GitHub Pull Requests, GitLab Merge Requestsなど)では、レビュアーがコードの品質や機能性を確認し、問題がなければLGTMを付与することで承認の意思を示します。
    • プロジェクトによっては、特定の数のLGTMが集まるとコードがマージ可能になる、といったルールが設けられています。
  2. Go言語のmap (マップ):

    • Go言語における組み込みのデータ構造の一つで、キーと値のペアを格納するコレクションです。他の言語では「ハッシュマップ」「辞書」「連想配列」などと呼ばれることもあります。
    • マップのキーは一意であるという特性があります。つまり、同じキーを複数回追加しようとしても、既存の値が上書きされるだけで、新しいエントリが追加されることはありません。この特性は、重複する要素を排除してユニークな要素のセットを作成する際に非常に有用です。
    • 構文例: m := make(map[string]int) は文字列をキー、整数を値とするマップを作成します。m["key"] = value で要素を追加・更新します。
  3. Go言語のslice (スライス):

    • Go言語における可変長シーケンス(配列のようなもの)です。内部的には固定長の配列を参照していますが、スライス自体は動的にサイズを変更できます。
    • append関数を使って要素を追加できます。slice = append(slice, element)のように使用し、必要に応じて新しい基底配列が割り当てられることがあります。
  4. コードレビューシステム (Rietveld/Gerrit):

    • Go言語プロジェクトは、初期にはGoogleが開発したRietveldというコードレビューツールを、その後Gerritというツールを利用していました。これらのシステムは、開発者が提出した変更(チェンジリスト、またはプルリクエスト)に対して、他の開発者がコメントを付けたり、承認(LGTM)を与えたりする機能を提供します。
    • これらのシステムは、APIを通じてレビューコメントや承認情報を取得できるため、本コミットで修正されているようなダッシュボードアプリケーションがそれらの情報を集約・表示することが可能になります。
  5. 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.Approvaltrue)である場合に、そのメッセージの送信者(s)を直接cl.LGTMsというスライスに追加していました。この方法では、同じ送信者が複数回承認した場合、その名前がcl.LGTMsに複数回現れてしまいます。

この問題を解決するために、以下の変更が導入されました。

  1. lgtmマップの導入:

    • lgtm := make(map[string]bool)という新しいマップが導入されました。このマップは、LGTMを付与したユニークなユーザー名をキーとして保持するために使用されます。値はbool型ですが、ここでは単にキーの存在を確認するために使われるため、trueで固定されます。
  2. LGTMの仮格納:

    • メッセージをループ処理する中で、msg.Approvaltrueの場合、以前は直接cl.LGTMssappendしていましたが、この変更ではlgtm[s] = trueとして、lgtmマップに送信者sを追加するように変更されました。マップのキーは一意であるため、同じ送信者が複数回現れても、マップにはその送信者のエントリが一つだけ保持されます。
  3. 重複排除後の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[s] = true (行322):

    • 上記の削除された行の代わりに、LGTM承認があった場合に送信者slgtmマップのキーとして追加する処理が挿入されました。マップの特性により、同じsが複数回追加されても、マップ内にはそのsに対応するエントリは一つしか存在しません。これにより、自動的に重複が排除されます。
  • + 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は現在アーカイブされていますが、当時の情報源として)