From 473a70b58fec9903e66f28dce2d562f89306e17d Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Mon, 22 Jun 2020 09:18:33 +0200 Subject: [PATCH] Don't add name/account ID to change messages Change messages are Git commits and have an author set. We use this author to serve the 'author' field on the REST API. The UI shows the author on the very left of the change screen. There is no need to mention this information in the message string in addition. In fact, doing so makes showing a nice message harder as the UI would have to parse out the account ID and resolve it. Change-Id: I71fe4101666b496064691f07e92c2b912f6fe2e7 --- .../gerrit/server/submit/SubmitStrategyOp.java | 18 ++---------------- .../acceptance/rest/change/AbstractSubmit.java | 4 ++-- 2 files changed, 4 insertions(+), 18 deletions(-) diff --git a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java index 5558c742ef..ab28aa9faf 100644 --- a/java/com/google/gerrit/server/submit/SubmitStrategyOp.java +++ b/java/com/google/gerrit/server/submit/SubmitStrategyOp.java @@ -23,7 +23,6 @@ import static java.util.Objects.requireNonNull; import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.data.SubmitRecord; -import com.google.gerrit.entities.Account; import com.google.gerrit.entities.BranchNameKey; import com.google.gerrit.entities.Change; import com.google.gerrit.entities.ChangeMessage; @@ -36,13 +35,11 @@ import com.google.gerrit.exceptions.StorageException; import com.google.gerrit.server.ApprovalsUtil; import com.google.gerrit.server.ChangeMessagesUtil; import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.account.AccountState; import com.google.gerrit.server.change.LabelNormalizer; import com.google.gerrit.server.git.CodeReviewCommit; import com.google.gerrit.server.git.CodeReviewCommit.CodeReviewRevWalk; import com.google.gerrit.server.git.GroupCollector; import com.google.gerrit.server.git.MergeUtil; -import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.project.ProjectConfig; import com.google.gerrit.server.project.ProjectState; @@ -394,24 +391,13 @@ abstract class SubmitStrategyOp implements BatchUpdateOp { } } - private String getByAccountName() { - requireNonNull(submitter, "getByAccountName called before submitter populated"); - Optional account = - args.accountCache.get(submitter.accountId()).map(AccountState::account); - if (account.isPresent() && account.get().fullName() != null) { - return " by " + ChangeNoteUtil.getAccountIdAsUsername(account.get().id()); - } - return ""; - } - private ChangeMessage message(ChangeContext ctx, CodeReviewCommit commit, CommitMergeStatus s) { requireNonNull(s, "CommitMergeStatus may not be null"); String txt = s.getDescription(); if (s == CommitMergeStatus.CLEAN_MERGE) { - return message(ctx, commit.getPatchsetId(), txt + getByAccountName()); + return message(ctx, commit.getPatchsetId(), txt); } else if (s == CommitMergeStatus.CLEAN_REBASE || s == CommitMergeStatus.CLEAN_PICK) { - return message( - ctx, commit.getPatchsetId(), txt + " as " + commit.name() + getByAccountName()); + return message(ctx, commit.getPatchsetId(), txt + " as " + commit.name()); } else if (s == CommitMergeStatus.SKIPPED_IDENTICAL_TREE) { return message(ctx, commit.getPatchsetId(), txt); } else if (s == CommitMergeStatus.ALREADY_MERGED) { diff --git a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index 2901361666..75950e2903 100644 --- a/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/javatests/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -1344,11 +1344,11 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { assertThat(messages).hasSize(3); String last = Iterables.getLast(messages); if (getSubmitType() == SubmitType.CHERRY_PICK) { - assertThat(last).startsWith("Change has been successfully cherry-picked as "); + assertThat(last).startsWith("Change has been successfully cherry-picked as"); } else if (getSubmitType() == SubmitType.REBASE_ALWAYS) { assertThat(last).startsWith("Change has been successfully rebased and submitted as"); } else { - assertThat(last).isEqualTo("Change has been successfully merged by Gerrit User 1000000"); + assertThat(last).isEqualTo("Change has been successfully merged"); } }