cmd/gerritbot: fix ID used to post comments with PR rule findings

Use the more modern "<project>~<changeNumber>" format for a change
and properly specify the first revision.

While we are here, we add a few comments to the gerrit package
to help avoid confusion in the future.

This is a follow-up to CL 513397, which was failing to post the
new PR rule findings for a test PR with error:

2023/08/15 22:06:38 processPullRequest: importGerritChangeFromPR(#71, nil): could not add findings comment to CL for #71: HTTP status 404 Not Found on request to https://go-review.googlesource.com/a/changes/If1a8ae9e4b76a05b13139ddf9fda1cdf67b50b33/revisions/build~master~If1a8ae9e4b76a05b13139ddf9fda1cdf67b50b33/review; Not found: build~master~If1a8ae9e4b76a05b13139ddf9fda1cdf67b50b33

Updates golang/go#61573

Change-Id: Ib0fef88ae05b10e623c2c1af614aa932dbfcc043
Reviewed-on: https://go-review.googlesource.com/c/build/+/519796
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: t hepudds <thepudds1460@gmail.com>

Author: thepudds

cmd/gerritbot/gerritbot.go

@@ -810,8 +810,8 @@ Please visit Gerrit at %s.
 					"/PATCHSET_LEVEL": {{Message: msg, Unresolved: &unresolved}},
 				},
 			}
-			// TODO: instead of gcl.ID, we might need to do something similar to changeTriple in cmd/coordinator.
-			err = b.gerritClient.SetReview(ctx, gcl.ChangeID, gcl.ID, ri)
+			changeID := fmt.Sprintf("%s~%d", url.PathEscape(gcl.Project), gcl.ChangeNumber)
+			err = b.gerritClient.SetReview(ctx, changeID, "1", ri)
 			if err != nil {
 				return fmt.Errorf("could not add findings comment to CL for %s: %v", prShortLink(pr), err)
 			}

gerrit/gerrit.go

@@ -221,13 +221,22 @@ const (
 // ChangeInfo is a Gerrit data structure.
 // See https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-info
 type ChangeInfo struct {
-	// ID is the ID of the change in the format
-	// "'<project>~<branch>~<Change-Id>'", where 'project',
-	// 'branch' and 'Change-Id' are URL encoded. For 'branch' the
-	// refs/heads/ prefix is omitted.
-	ID           string `json:"id"`
-	ChangeNumber int    `json:"_number"`
-	ChangeID     string `json:"change_id"`
+	// The ID of the change. Subject to a 'GerritBackendFeature__return_new_change_info_id' experiment,
+	// the format is either "'<project>~<_number>'" (new format),
+	// or "'<project>~<branch>~<Change-Id>'" (old format).
+	// 'project', '_number', and 'branch' are URL encoded.
+	// For 'branch' the refs/heads/ prefix is omitted.
+	// The callers must not rely on the format.
+	ID string `json:"id"`
+
+	// ChangeNumber is a change number like "4247".
+	ChangeNumber int `json:"_number"`
+
+	// ChangeID is the Change-Id footer value like "I8473b95934b5732ac55d26311a706c9c2bde9940".
+	// Note that some of the functions in this package take a changeID parameter that is a {change-id},
+	// which is a distinct concept from a Change-Id footer. (See the documentation links for details,
+	// including https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#change-id).
+	ChangeID string `json:"change_id"`
 
 	Project string `json:"project"`