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

このコミットは、Goプロジェクトのコードレビューツールであるcodereview.pyから、未使用のupload_options.revisionというオプションを削除するものです。これにより、コードベースの整理と不要なロジックの削除が行われ、ツールの保守性が向上します。特に、Mercurial (hg) バージョン管理システムにおけるベースリビジョンの決定ロジックが簡素化されています。

コミット

• コミットハッシュ: dd449a1cfa02c908393c927920474c1fe017558f
• Author: Patrick Mézard patrick@mezard.eu
• Date: Mon Feb 24 10:11:37 2014 -0500

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

https://github.com/golang/go/commit/dd449a1cfa02c908393c927920474c1fe017558f

元コミット内容

codereview: remove unused upload_options.revision

LGTM=rsc
R=golang-codereviews, rsc
CC=golang-codereviews
https://golang.org/cl/60640052

変更の背景

この変更の背景には、Goプロジェクトが当時使用していたコードレビューシステム（Rietveldベース）のcodereview.pyスクリプトの内部的なクリーンアップがあります。upload_options.revisionというオプションは、コード変更をアップロードする際に、どのリビジョンをベースとして差分を生成するかを指定するために設計された可能性があります。しかし、実際の運用やコードパスにおいて、このオプションが一度も使用されていない、あるいはその機能が他の方法で代替されていたため、不要なコードとして残っていました。

未使用のコードは、可読性を低下させ、将来的なメンテナンスの負担となるだけでなく、誤解を招く可能性もあります。そのため、開発者はコードベースを健全に保つために、このようなデッドコードを定期的に削除します。このコミットは、まさにその目的のために行われたものです。

前提知識の解説

Goプロジェクトのコードレビュープロセス (当時)

Goプロジェクトは、初期にはGoogleが開発したRietveldというコードレビューツールを使用していました。codereview.pyスクリプトは、このRietveldシステムと連携し、開発者がローカルの変更をRietveldサーバーにアップロードし、レビュープロセスを開始するためのCLIツールとして機能していました。

Rietveld

Rietveldは、GoogleのソフトウェアエンジニアであるGuido van Rossum（Pythonの作者）によって開発された、Webベースのコードレビューシステムです。PerforceやSubversionなどのバージョン管理システムと連携し、差分表示、コメント、承認などの機能を提供します。Goプロジェクトでは、Mercurialと連携して使用されていました。

Mercurial (hg)

Mercurialは、分散型バージョン管理システム（DVCS）の一つで、Gitと同様にリポジトリのクローンをローカルに持ち、オフラインでの作業が可能です。Goプロジェクトは、初期にはMercurialを主要なバージョン管理システムとして採用していました。

upload_options

codereview.pyスクリプト内で、コード変更をRietveldにアップロードする際の様々な設定やオプションを保持するためのデータ構造（おそらくオブジェクトや辞書）です。これには、コミットメッセージ、イシュー番号、そしてこのコミットで削除されるrevisionなどの情報が含まれていました。

MercurialVCS クラス

codereview.py内でMercurialリポジトリとのインタラクションを抽象化するためのクラスです。このクラスは、リポジトリのパスの取得、ファイルの差分生成、そして変更のベースとなるリビジョン（base_rev）の決定など、Mercurial固有の操作をカプセル化していました。

base_rev

base_revは、コードレビューのために差分を生成する際の「基準となるリビジョン」を指します。例えば、あるブランチの最新コミットから作業を開始し、いくつかの変更を加えた場合、その変更の差分は、作業を開始した時点のコミット（base_rev）と比較して生成されます。

hg log --rev qparent --template={node}

Mercurialコマンドの一つです。

• hg log: リビジョン履歴を表示します。
• --rev qparent: 現在の作業ディレクトリがMQ (Mercurial Queues) パッチの適用中である場合、そのパッチが適用されているベースリビジョン（qparent）を指定します。MQは、Mercurialでパッチを管理するための拡張機能です。
• --template={node}: 出力フォーマットを指定します。ここでは、リビジョンのノードID（ハッシュ値）のみを出力するように指定しています。 このコマンドは、MQパッチが適用されている場合に、そのパッチのベースとなっているリビジョンハッシュを取得するために使用されます。

hg parents -q

Mercurialコマンドの一つです。

• hg parents: 現在の作業ディレクトリの親リビジョンを表示します。
• -q (quiet): 詳細な情報を表示せず、親リビジョンのノードID（ハッシュ値）のみを簡潔に表示します。 このコマンドは、現在の作業ディレクトリがどのリビジョンから派生しているか、つまりそのベースとなるリビジョンを取得するために使用されます。

技術的詳細

このコミットの主要な変更は、lib/codereview/codereview.pyファイル内のRietveldSetup関数とMercurialVCSクラスに集中しています。

RietveldSetup関数における変更

以前のコードでは、RietveldSetup関数内でupload_optionsオブジェクトの初期化時にupload_options.revision = Noneという行がありました。これは、revisionオプションがデフォルトでNoneに設定されることを意味します。このコミットでは、この行が完全に削除されています。これは、upload_options.revisionがコードの他の場所で参照されることがなく、完全に未使用であったため、初期化自体が不要になったことを示しています。

MercurialVCSクラスにおける変更

MercurialVCSクラスの__init__メソッド内で、self.base_revを決定するロジックが変更されています。

変更前:

        if self.options.revision:
            self.base_rev = self.options.revision
        else:
            mqparent, err = RunShellWithReturnCode(['hg', 'log', '--rev', 'qparent', '--template={node}'])
            if not err and mqparent != "":
                self.base_rev = mqparent
            else:
                out = RunShell(["hg", "parents", "-q"], silent_ok=True).strip()
                if not out:
                    # No revisions; use 0 to mean a repository with nothing.
                    out = "0:0"
                self.base_rev = out.split(':')[1].strip()

このロジックは、以下の優先順位でself.base_revを決定していました。

1. self.options.revisionが設定されていれば、それを使用する。
2. そうでなければ、hg log --rev qparentコマンドを実行し、MQパッチのベースリビジョンを取得する。
3. それも失敗した場合（MQパッチが適用されていないなど）、hg parents -qコマンドを実行し、現在の作業ディレクトリの親リビジョンを取得する。リビジョンが存在しない場合は"0:0"を使用する。

変更後:

        mqparent, err = RunShellWithReturnCode(['hg', 'log', '--rev', 'qparent', '--template={node}'])
        if not err and mqparent != "":
            self.base_rev = mqparent
        else:
            out = RunShell(["hg", "parents", "-q"], silent_ok=True).strip()
            if not out:
                # No revisions; use 0 to mean a repository with nothing.
                out = "0:0"
            self.base_rev = out.split(':')[1].strip()

変更後では、if self.options.revision:によるチェックが完全に削除されています。これは、self.options.revisionが常にNoneであるか、あるいは設定されていてもこのロジックに影響を与えないことが確認されたためです。結果として、self.base_revの決定は、まずMQパッチのベースリビジョンを試み、それが失敗した場合にhg parentsコマンドで親リビジョンを取得するという、より直接的なロジックに簡素化されました。

この変更は、コードの冗長性を排除し、base_revの決定ロジックをより明確にすることで、codereview.pyの保守性を向上させています。

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

--- a/lib/codereview/codereview.py
+++ b/lib/codereview/codereview.py
@@ -2656,7 +2656,6 @@ def RietveldSetup(ui, repo):
 	upload_options.message = None
 	upload_options.issue = None
 	upload_options.download_base = False
-	upload_options.revision = None
 	upload_options.send_mail = False
 	upload_options.vcs = None
 	upload_options.server = server
@@ -3435,18 +3434,16 @@ class MercurialVCS(VersionControlSystem):\
 		cwd = os.path.normpath(os.getcwd())
 		assert cwd.startswith(self.repo_dir)
 		self.subdir = cwd[len(self.repo_dir):].lstrip(r"\/")
-		if self.options.revision:
-			self.base_rev = self.options.revision
-		else:
-			mqparent, err = RunShellWithReturnCode(['hg', 'log', '--rev', 'qparent', '--template={node}'])
-			if not err and mqparent != "":
-				self.base_rev = mqparent
-			else:
-				out = RunShell(["hg", "parents", "-q"], silent_ok=True).strip()
-				if not out:
-					# No revisions; use 0 to mean a repository with nothing.
-					out = "0:0"
-				self.base_rev = out.split(':')[1].strip()
+		mqparent, err = RunShellWithReturnCode(['hg', 'log', '--rev', 'qparent', '--template={node}'])
+		if not err and mqparent != "":
+			self.base_rev = mqparent
+		else:
+			out = RunShell(["hg", "parents", "-q"], silent_ok=True).strip()
+			if not out:
+				# No revisions; use 0 to mean a repository with nothing.
+				out = "0:0"
+			self.base_rev = out.split(':')[1].strip()
+
 	def _GetRelPath(self, filename):
 		"""Get relative path of a file according to the current directory,
 		given its logical path in the repo."""

コアとなるコードの解説

lib/codereview/codereview.py

RietveldSetup 関数 (行 2656-2657)

• - upload_options.revision = None: この行が削除されました。
  • これは、upload_optionsオブジェクトのrevision属性が初期化時にNoneに設定されていた部分です。この属性がコードの他の場所で実際に使用されていないことが判明したため、この初期化の行自体が不要と判断され、削除されました。これにより、コードの冗長性が減り、よりクリーンになります。

MercurialVCS クラスの __init__ メソッド (行 3435-3452)

• - if self.options.revision: から始まるブロックが削除され、その内部の else ブロックが直接実行されるように変更されました。
  • 変更前:
    • まず、self.options.revisionが設定されているかどうかをチェックしていました。もし設定されていれば、その値をself.base_revとして使用していました。
    • self.options.revisionが設定されていない場合（またはNoneの場合）にのみ、hg log --rev qparentコマンドを実行してMQパッチのベースリビジョンを取得しようとしていました。
    • hg log --rev qparentが成功しなかった場合、最終手段としてhg parents -qコマンドを実行して現在の作業ディレクトリの親リビジョンを取得していました。
  • 変更後:
    • self.options.revisionのチェックが完全に削除されました。これは、このオプションが実際には使用されていなかったか、あるいは常にNoneであったため、この条件分岐が常にelseパスにフォールバックしていたことを示唆しています。
    • 結果として、コードは直接hg log --rev qparentコマンドの実行から開始され、その結果に基づいてself.base_revを設定します。
    • もしhg log --rev qparentが失敗した場合、以前と同様にhg parents -qコマンドにフォールバックして親リビジョンを取得します。
  • この変更により、base_revを決定するロジックが簡素化され、未使用のコードパスが削除されました。これにより、コードの意図がより明確になり、将来的なメンテナンスが容易になります。

関連リンク

• Go Gerrit Change-ID: https://golang.org/cl/60640052

参考にした情報源リンク

• Mercurial Documentation: https://www.mercurial-scm.org/
• Rietveld (Wikipedia): https://en.wikipedia.org/wiki/Rietveld
• Mercurial Queues (MQ): https://www.mercurial-scm.org/wiki/MqExtension
• Go project's transition from Mercurial to Git (relevant for historical context): https://go.dev/blog/git (Although this commit predates the full transition, it provides context on Go's VCS history.)