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

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

このコミットは、Go言語のコードレビューダッシュボードシステムにおける表示ロジックの改善を目的としています。具体的には、コードレビュー(CL: Change List)の最終更新からの経過時間をより簡潔な形式で表示するように変更し、またCLのオーナー表示に関する関数名をより明確なものに修正しています。

コミット

  • コミットハッシュ: 1a7905372536154661b094accadcbc1e692b1544
  • 作者: David Symonds dsymonds@golang.org
  • コミット日時: 2012年4月27日 金曜日 17:12:09 +1000

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

https://github.com/golang/go/commit/1a7905372536154661b094accadcbc1e692b1544

元コミット内容

misc/dashboard/codereview: more abbreviated modification duration.

R=golang-dev, r
CC=golang-dev
https://golang.org/cl/6131044

変更の背景

Go言語のコードレビューダッシュボードは、多数のコードレビューを一覧表示し、その状態を把握するためのツールです。以前のバージョンでは、CLの最終更新からの経過時間(ModifiedAgo)が詳細な文字列形式で表示されていました。しかし、ダッシュボードのような一覧性の高いUIでは、情報が冗長になりすぎると視認性が低下します。このコミットの背景には、ユーザーがより迅速に情報を把握できるよう、経過時間の表示を「より簡潔な(more abbreviated)」形式に改善するという目的があります。例えば、「1時間30分前」ではなく「1h」のように表示することで、一目で情報を理解できるようにします。

また、ShortOwnerという関数名が、その実態(CLのオーナーをメールアドレスまたは個人IDとして表示する)を十分に表していないという問題意識があったと考えられます。表示目的であることを明確にするため、より適切なDisplayOwnerへのリネームが行われました。

前提知識の解説

Go言語のtimeパッケージ

Go言語の標準ライブラリであるtimeパッケージは、時間と期間を扱うための強力な機能を提供します。

  • time.Time: 特定の時点を表す型です。
  • time.Duration: 期間(時間の長さ)を表す型です。ナノ秒単位で内部的に保持されます。
  • time.Now(): 現在のローカル時刻をtime.Time型で返します。
  • time.Time.Sub(t time.Time) time.Duration: 2つのtime.Time間の差をtime.Durationとして返します。
  • time.Duration.String(): time.Durationを人間が読める文字列形式(例: "1h30m0s")に変換します。

Go言語のfmtパッケージ

fmtパッケージは、フォーマットされたI/O(入出力)を実装します。

  • fmt.Sprintf(format string, a ...interface{}) string: フォーマット文字列と引数に基づいて文字列を生成し、その文字列を返します。C言語のsprintfに似ています。

Go言語のhtml/templateパッケージ

html/templateパッケージは、HTML出力の生成を安全に行うためのテンプレートエンジンを提供します。クロスサイトスクリプティング(XSS)攻撃を防ぐために、自動的にエスケープ処理を行います。

  • template.HTML: この型にキャストされた文字列は、テンプレートエンジンによってHTMLとして安全であると見なされ、エスケープされずにそのまま出力されます。

コードレビューシステムとダッシュボード

コードレビューシステムは、ソフトウェア開発において、コードの品質向上、バグの早期発見、知識共有などを目的として、他の開発者が書いたコードをレビューするプロセスを支援するツールです。 ダッシュボードは、これらのコードレビューの現在の状態(未レビュー、レビュー中、承認済みなど)や、最終更新からの経過時間、担当者などの情報を一覧で表示するUIです。一目で多くの情報を把握できることが重要であり、そのためには情報の表示形式が簡潔であることが求められます。

技術的詳細

このコミットは、主にmisc/dashboard/codereview/dashboard/cl.gomisc/dashboard/codereview/dashboard/front.goの2つのファイルに変更を加えています。

cl.goの変更点

  1. ShortOwnerからDisplayOwnerへの関数名変更:

    • CL構造体のメソッドShortOwner()DisplayOwner()にリネームされました。
    • この変更は、関数の目的(CLのオーナー情報を表示用に整形して返す)をより明確にするためのものです。機能的な変更はありません。
  2. ModifiedAgo()関数のロジック変更:

    • この関数は、CLの最終更新時刻(cl.Modified)から現在時刻までの経過時間を計算し、その期間を文字列として返します。
    • 変更前:
      • time.Now().Sub(cl.Modified)で期間dを計算。
      • d -= d % time.Minuteで分単位に切り捨て。
      • d.String()で期間を文字列化(例: "1h30m0s")。
      • 末尾が"0s"であれば削除(例: "1h30m")。
    • 変更後:
      • 期間dを計算するところまでは同じ。
      • unitsというmap[string]time.Durationを定義し、"d"(日)、"h"(時間)、"m"(分)、"s"(秒)に対応するtime.Duration値を格納。
      • このunitsマップをイテレートし、期間dが各単位uよりも大きいかどうかをチェックします。
      • 最初にd > uを満たした単位で、fmt.Sprintf("%d%s", d/u, suffix)を使って「数値+単位の略称」形式の文字列を生成し、それを返します。例えば、期間が1時間30分であれば、d > time.Hourが最初に真となり、「1h」のような形式で返されます。
      • どの単位にも当てはまらない(つまり、期間が非常に短い)場合は、"just now"(たった今)を返します。
      • この新しいロジックにより、表示される期間は常に最も大きい単位で簡潔に表現されるようになります。例えば、1日と3時間であれば「1d」、2時間45分であれば「2h」、30秒であれば「30s」となります。

front.goの変更点

  1. テンプレートの更新:
    • HTMLテンプレート内で$cl.ShortOwnerが呼び出されていた箇所が$cl.DisplayOwnerに修正されました。
    • これはcl.goでの関数名変更に対応するもので、ダッシュボードのフロントエンドが正しい関数を呼び出すようにするための変更です。

これらの変更により、ダッシュボード上でのCLの更新経過時間がより簡潔に表示され、視認性が向上しました。また、関数名がより意図を明確に伝えるものになりました。

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

misc/dashboard/codereview/dashboard/cl.go

--- a/misc/dashboard/codereview/dashboard/cl.go
+++ b/misc/dashboard/codereview/dashboard/cl.go
@@ -47,9 +47,9 @@ type CL struct {
 	Reviewer string
 }
 
-// ShortOwner returns the CL's owner, either as their email address
+// DisplayOwner returns the CL's owner, either as their email address
 // or the person ID if it's a reviewer. It is for display only.
-func (cl *CL) ShortOwner() string {
+func (cl *CL) DisplayOwner() string {
 	if p, ok := emailToPerson[cl.Owner]; ok {
 		return p
 	}
@@ -79,13 +79,20 @@ func (cl *CL) LGTMHTML() template.HTML {
 }
 
 func (cl *CL) ModifiedAgo() string {
+// Just the first non-zero unit.
+	units := map[string]time.Duration{
+		"d": 24 * time.Hour,
+		"h": time.Hour,
+		"m": time.Minute,
+		"s": time.Second,
+	}
 	d := time.Now().Sub(cl.Modified)
-	d -= d % time.Minute // truncate to minute resolution
-	s := d.String()
-	if strings.HasSuffix(s, "0s") {
-		s = s[:len(s)-2]
+	for suffix, u := range units {
+		if d > u {
+			return fmt.Sprintf("%d%s", d/u, suffix)
+		}
 	}
-	return s
+	return "just now"
 }
 
 func handleAssign(w http.ResponseWriter, r *http.Request) {

misc/dashboard/codereview/dashboard/front.go

--- a/misc/dashboard/codereview/dashboard/front.go
+++ b/misc/dashboard/codereview/dashboard/front.go
@@ -192,7 +192,7 @@ var frontPage = template.Must(template.New("front").Funcs(template.FuncMap{
 <table class="cls">
 {{range $cl := .CLs}}
   <tr id="cl-{{$cl.Number}}">
-    <td class="email">{{$cl.ShortOwner}}</td>
+    <td class="email">{{$cl.DisplayOwner}}</td>
     {{if $tbl.Assignable}}
     <td>
     <select id="cl-rev-{{$cl.Number}}" {{if not $.UserIsReviewer}}disabled{{end}}>

コアとなるコードの解説

cl.goModifiedAgo()関数

この関数の変更が、このコミットの主要な機能改善点です。

func (cl *CL) ModifiedAgo() string {
	// Just the first non-zero unit.
	units := map[string]time.Duration{
		"d": 24 * time.Hour,
		"h": time.Hour,
		"m": time.Minute,
		"s": time.Second,
	}
	d := time.Now().Sub(cl.Modified) // 現在時刻とCLの最終更新時刻の差を計算
	for suffix, u := range units {   // 定義された単位(日、時、分、秒)を大きい順にイテレート
		if d > u {                   // もし期間dが現在の単位uよりも大きければ
			return fmt.Sprintf("%d%s", d/u, suffix) // その単位でフォーマットして返す
		}
	}
	return "just now" // どの単位よりも小さければ「just now」を返す
}

この新しいロジックは、以下の点で優れています。

  1. 簡潔性: 期間を最も大きい適切な単位で表示するため、例えば「1日と3時間」は「1d」となり、非常に簡潔です。
  2. 可読性: ダッシュボードのような一覧表示では、短い文字列の方が一目で理解しやすくなります。
  3. 効率性: 以前のd.String()からの文字列操作(末尾の"0s"削除)が不要になり、より直接的に目的の形式を生成します。

unitsマップは、表示の優先順位を暗黙的に定義しています。マップのイテレーション順序は保証されませんが、Goのマップは通常、挿入順序を保持しないため、このコードが意図通りに「日」→「時」→「分」→「秒」の順でチェックされることを保証するためには、unitsをスライスで定義し、明示的に順序を制御する方がより堅牢です。しかし、この特定のケースでは、d > uの条件が最初に満たされるものが最も大きい単位となるため、マップのイテレーション順序が結果に影響を与えることはありません。

cl.goShortOwner()からDisplayOwner()へのリネーム

// ShortOwner returns the CL's owner, either as their email address
// or the person ID if it's a reviewer. It is for display only.
func (cl *CL) ShortOwner() string {
// ...
}

// DisplayOwner returns the CL's owner, either as their email address
// or the person ID if it's a reviewer. It is for display only.
func (cl *CL) DisplayOwner() string {
// ...
}

に変更されました。これはセマンティックな変更であり、コードの振る舞いには影響しませんが、関数の意図をより明確にすることで、将来のコードの保守性や可読性を向上させます。コメントも同時に更新され、「It is for display only.」という記述が残されていることから、この関数が内部的なロジックではなく、UI表示のために特化していることが強調されています。

front.goのテンプレート変更

<td class="email">{{$cl.ShortOwner}}</td>

<td class="email">{{$cl.DisplayOwner}}</td>

に変更されました。これはcl.goでの関数名変更に合わせた単純な修正であり、フロントエンドが新しい関数名でCLオーナー情報を取得するようにします。

これらの変更は全体として、Goコードレビューダッシュボードのユーザーエクスペリエンスを向上させ、コードベースの可読性と保守性を高めるための、小さくも効果的な改善です。

関連リンク

参考にした情報源リンク