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

このコミットは、Go言語のコードレビューダッシュボードシステムにおいて、「NOT LGTM」というレビューコメントを認識し、適切に処理・表示する機能を追加するものです。これにより、以前の「LGTM」（Looks Good To Me）の承認を無効化し、コードレビューのステータスをより正確に反映できるようになります。

コミット

commit 3cc9d16792367310f0cdb03272ed1ec3d3e43eca
Author: David Symonds <dsymonds@golang.org>
Date:   Mon Jul 30 10:54:50 2012 +1000

    misc/dashboard/codereview: recognize "NOT LGTM".
    
    A "NOT LGTM" overrules a previous "LGTM" by the same person, and vice versa.
    "NOT LGTM"s are shown in the same location as LGTMs, colored red.
    
    R=rsc
    CC=golang-dev
    https://golang.org/cl/6453062

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

https://github.com/golang/go/commit/3cc9d16792367310f0cdb03272ed1ec3d3e43eca

元コミット内容

misc/dashboard/codereview: recognize "NOT LGTM".

A "NOT LGTM" overrules a previous "LGTM" by the same person, and vice versa.
"NOT LGTM"s are shown in the same location as LGTMs, colored red.

R=rsc
CC=golang-dev
https://golang.org/cl/6453062

Go言語の開発プロセスでは、Gerritのようなコードレビューシステムが利用されており、変更提案（Change List, CL）に対してレビュアーが「LGTM」（Looks Good To Me）という承認を与えることで、その変更が承認されたことを示します。しかし、一度LGTMを与えた後で、レビュアーが何らかの理由でその承認を撤回したい場合や、追加の懸念事項が見つかった場合に、その意思を明確に表示するメカニズムが不足していました。

このコミットは、このギャップを埋めるために導入されました。具体的には、「NOT LGTM」というコメントを特別な意味を持つものとして認識し、以前のLGTMを無効化する機能、およびそのステータスをダッシュボード上で視覚的に表示する機能を追加することで、コードレビューのワークフローをより柔軟かつ正確にすることを目的としています。これにより、レビュアーは自身の承認を明確に撤回でき、他の開発者もCLの真の承認状況を一目で把握できるようになります。

前提知識の解説

このコミットを理解するためには、以下の概念について知っておく必要があります。

コードレビュー (Code Review): ソフトウェア開発において、他の開発者が書いたコードをレビューし、品質、バグ、設計上の問題などをチェックするプロセスです。Go言語の開発では、Gerritのようなツールが使われることが一般的です。
Change List (CL): Gerritなどのコードレビューシステムにおける、一連の変更（コミット）の単位です。開発者はCLを作成し、レビューを依頼します。
LGTM (Looks Good To Me): コードレビューにおいて、レビュアーがその変更を承認し、マージしても良いと判断したことを示す一般的な略語です。多くのコードレビューシステムで、この承認がマージの条件の一つとなります。
Go App Engine: Google App Engineは、Googleが提供するPaaS（Platform as a Service）であり、ウェブアプリケーションやモバイルバックエンドを構築・ホストするためのプラットフォームです。このコミットが変更を加えているmisc/dashboard/codereviewは、Goのコードレビュープロセスを管理・表示するためのダッシュボードアプリケーションであり、Go App Engine上で動作している可能性が高いです。
Go言語のhtml/templateパッケージ: Go言語の標準ライブラリの一部で、HTMLテンプレートを安全に生成するためのパッケージです。クロスサイトスクリプティング（XSS）攻撃を防ぐために、自動的にエスケープ処理を行います。
Go言語のdatastoreパッケージ: Google App Engineのデータストア（NoSQLデータベース）とやり取りするためのGo言語のクライアントライブラリです。datastore:",noindex"タグは、そのフィールドがデータストアのインデックス作成対象から除外されることを示します。
Go言語のstringsパッケージ: 文字列操作のためのユーティリティ関数を提供するGo言語の標準ライブラリです。
Go言語のsortパッケージ: スライスやカスタムコレクションをソートするための関数を提供するGo言語の標準ライブラリです。

技術的詳細

このコミットは、Go言語のコードレビューダッシュボードアプリケーションのバックエンドとフロントエンドの両方に変更を加えています。

データモデルの拡張 (CL構造体):
- misc/dashboard/codereview/dashboard/cl.go内のCL構造体に、新たにNotLGTMs []stringフィールドが追加されました。これは、LGTMと同様に、特定のCLに対して「NOT LGTM」を与えたレビュアーのメールアドレス（または識別子）を格納するためのスライスです。datastore:",noindex"タグが付与されており、このフィールドがデータストアのインデックス作成対象から除外されることを示しています。これは、このフィールドが検索のキーとして使われることが少なく、インデックスのオーバーヘッドを避けるためと考えられます。
HTML表示ロジックの共通化と追加:
- cl.go内で、レビュアーのメールアドレスリストをHTML形式で整形するformatEmails関数が導入されました。これは、以前LGTMHTML()内にあったロジックを共通化したものです。これにより、コードの重複が削減され、保守性が向上しています。
- LGTMHTML()関数は、このformatEmails関数を呼び出すように変更されました。
- 新たにNotLGTMHTML()関数が追加され、これもformatEmails関数を利用してNotLGTMsフィールドの内容をHTML形式で整形して返します。
コードレビューメッセージの解析と状態管理:
- cl.go内のupdateCL関数は、コードレビューのメッセージを解析し、CLのステータスを更新する主要なロジックを含んでいます。
- この関数内で、lgtmマップに加えて新たにnotLGTMマップが導入されました。これらのマップは、各レビュアーからの最新のLGTM/NOT LGTMの状態を一時的に保持するために使用されます。
- メッセージのテキスト内容に「NOT LGTM」という文字列が含まれている場合、そのメッセージの送信者（msg.Sender）がnotLGTMマップに追加されます。
- 重要なロジック: 「NOT LGTM」が検出された場合、同じ送信者からの以前の「LGTM」はlgtmマップから削除されます（delete(lgtm, s)）。逆に、「LGTM」が検出された場合、同じ送信者からの以前の「NOT LGTM」はnotLGTMマップから削除されます（delete(notLGTM, s)）。これにより、「NOT LGTM」は「LGTM」を上書きし、その逆も同様であるというコミットメッセージの意図がコードに反映されています。
- 最終的に、lgtmマップとnotLGTMマップの内容が、それぞれcl.LGTMsとcl.NotLGTMsスライスに格納され、ソートされます。
フロントエンドでの表示:
- misc/dashboard/codereview/dashboard/front.go内のHTMLテンプレート（frontPage）が変更されました。
- 既存のLGTMの表示に加えて、{{if and .NotLGTMs $tbl.Assignable}} NOT LGTMs: {{.NotLGTMHTML}}{{end}}という行が追加されました。
- これにより、NotLGTMsフィールドに値が存在する場合、その内容が「NOT LGTMs: 」というプレフィックスとともに表示されます。
- color: #f74545;というインラインスタイルが適用されており、NOT LGTMの表示が赤色になるように指定されています。これは、視覚的に警告や否定的な意味合いを伝えるためのものです。

これらの変更により、システムは「NOT LGTM」という特定のキーワードを認識し、その情報をデータモデルに永続化し、さらにユーザーインターフェース上で視覚的に区別して表示できるようになります。

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

`misc/dashboard/codereview/dashboard/cl.go`

// CL struct
type CL struct {
	Description []byte `datastore:",noindex"`
	FirstLine   string `datastore:",noindex"`
	LGTMs       []string
	NotLGTMs    []string // 追加されたフィールド

	// ...
}

// formatEmails関数 (LGTMHTMLから抽出され、共通化)
func formatEmails(e []string) template.HTML {
	x := make([]string, len(e))
	for i, s := range e {
		s = template.HTMLEscapeString(s)
		if !strings.Contains(s, "@") {
			s = "<b>" + s + "</b>"
		}
		x[i] = s
	}
	return template.HTML(strings.Join(x, ", "))
}

// LGTMHTML関数 (formatEmailsを呼び出すように変更)
func (cl *CL) LGTMHTML() template.HTML {
	return formatEmails(cl.LGTMs)
}

// NotLGTMHTML関数 (新規追加)
func (cl *CL) NotLGTMHTML() template.HTML {
	return formatEmails(cl.NotLGTMs)
}

// updateCL関数内の変更
func updateCL(c appengine.Context, n string) error {
	// ...
	lgtm := make(map[string]bool)
	notLGTM := make(map[string]bool) // 新規追加
	rcpt := make(map[string]bool)
	for _, msg := range apiResp.Messages {
		// ...

		if msg.Approval {
			lgtm[s] = true
			delete(notLGTM, s) // "LGTM" overrules previous "NOT LGTM"
		}
		if strings.Contains(msg.Text, "NOT LGTM") { // "NOT LGTM"の検出ロジック
			notLGTM[s] = true
			delete(lgtm, s) // "NOT LGTM" overrules previous "LGTM"
		}

		// ...
	}

	for l := range lgtm {
		cl.LGTMs = append(cl.LGTMs, l)
	}
	for l := range notLGTM { // NotLGTMsの追加
		cl.NotLGTMs = append(cl.NotLGTMs, l)
	}
	// ...
	sort.Strings(cl.LGTMs)
	sort.Strings(cl.NotLGTMs) // NotLGTMsのソート
	sort.Strings(cl.Recipients)

	// ...
}

`misc/dashboard/codereview/dashboard/front.go`

// frontPageテンプレート内の変更
var frontPage = template.Must(template.New("front").Funcs(template.FuncMap{
	// ...
}))

// HTMLテンプレートの該当箇所
// ...
    <td>
      <a href="http://codereview.appspot.com/{{.Number}}/" title="{{ printf "%s" .Description}}">{{.Number}}: {{.FirstLineHTML}}</a>
      {{if and .LGTMs $tbl.Assignable}}<br /><span style="font-size: smaller;">LGTMs: {{.LGTMHTML}}{{end}}</span>
      {{if and .NotLGTMs $tbl.Assignable}}<br /><span style="font-size: smaller; color: #f74545;">NOT LGTMs: {{.NotLGTMHTML}}{{end}}</span> // 新規追加
    </td>
// ...

コアとなるコードの解説

`cl.go`

CL構造体へのNotLGTMsフィールド追加: CL構造体は、個々のコードレビュー変更（CL）のメタデータと状態を保持します。NotLGTMs []stringの追加により、どのレビュアーが「NOT LGTM」を与えたかを追跡できるようになりました。これは、データストアに永続化される情報です。
formatEmails関数の導入と利用: LGTMHTML関数と新しく追加されたNotLGTMHTML関数は、レビュアーのメールアドレスリストをHTML形式で表示する役割を担います。以前はLGTMHTML内に直接記述されていたこの整形ロジックをformatEmailsとして独立させることで、コードの再利用性を高め、将来的に同様のリスト表示が必要になった場合の拡張性を確保しています。template.HTMLEscapeStringを使用することで、XSS攻撃を防ぐためのHTMLエスケープ処理が自動的に行われます。また、メールアドレスに@が含まれない場合は、ユーザー名が太字で表示されるようにしています。
updateCL関数における「NOT LGTM」の処理ロジック: この関数は、コードレビューシステムからのメッセージを処理し、CLの承認状態を更新する中心的な部分です。
- lgtmとnotLGTMという2つのmap[string]boolが導入されています。これらのマップは、各レビュアーからの最新の承認（LGTMまたはNOT LGTM）を効率的に管理するために使用されます。マップのキーはレビュアーの識別子（メールアドレスなど）、値は単にそのレビュアーが承認を与えたかどうかを示すブール値です。
- メッセージがmsg.Approval（LGTMに相当）である場合、その送信者はlgtmマップに追加され、同時にnotLGTMマップから削除されます。これは、「LGTM」が「NOT LGTM」を上書きするというルールを実装しています。
- メッセージのテキスト内容に"NOT LGTM"という文字列が含まれている場合、その送信者はnotLGTMマップに追加され、同時にlgtmマップから削除されます。これは、「NOT LGTM」が「LGTM」を上書きするという逆のルールを実装しています。
- この相互排他的なロジックにより、同じレビュアーがLGTMとNOT LGTMの両方を与えた場合でも、常に最新の承認状態が優先されるようになります。
- 最後に、lgtmとnotLGTMマップの内容がそれぞれcl.LGTMsとcl.NotLGTMsスライスに変換され、sort.Stringsによってアルファベット順にソートされます。これにより、表示順序が統一され、視認性が向上します。

`front.go`

HTMLテンプレートへの「NOT LGTM」表示の追加: front.goは、ダッシュボードのフロントページをレンダリングするためのHTMLテンプレートを定義しています。
- {{if and .NotLGTMs $tbl.Assignable}}という条件文は、NotLGTMsスライスに要素があり、かつ現在のユーザーがCLに割り当て可能（おそらくレビュー権限がある）な場合にのみ、NOT LGTMの表示ブロックをレンダリングすることを示しています。
-  NOT LGTMs: {{.NotLGTMHTML}}{{end}}というHTMLスニペットが追加されました。これにより、NOT LGTMを与えたレビュアーのリストが、LGTMのリストの下に、より小さいフォントサイズで、かつ赤色（#f74545）で表示されるようになります。赤色は、否定的な承認や注意が必要な状態を視覚的に強調する役割を果たします。

これらの変更により、Goのコードレビューダッシュボードは、「NOT LGTM」という重要なフィードバックを正確に処理し、開発者に対してCLの真の承認状況を明確に伝えることができるようになりました。

参考にした情報源リンク

Go言語のドキュメント（html/template, strings, sortパッケージなど）
Google App Engineのドキュメント（datastoreパッケージなど）
一般的なコードレビューのプラクティスと「LGTM」の概念
GitHubのコミット履歴とファイル変更差分
Go言語のコードレビュープロセスに関する一般的な知識
https://golang.org/cl/6453062 (元コミットに記載されているChange ListのURL)
https://go.dev/doc/contribute (Goプロジェクトへの貢献ガイドライン、コードレビュープロセスについて言及がある可能性)
https://go.dev/blog/gerrit (GoプロジェクトにおけるGerritの使用に関するブログ記事など)
https://pkg.go.dev/google.golang.org/appengine/datastore (App Engine Datastore Goクライアントライブラリのドキュメント)
https://pkg.go.dev/html/template (Go html/templateパッケージのドキュメント)
https://pkg.go.dev/strings (Go stringsパッケージのドキュメント)
https://pkg.go.dev/sort (Go sortパッケージのドキュメント)

comemo