From da4bbc42477ba04d175cc0775a0c5ec90c4c24fe Mon Sep 17 00:00:00 2001 From: Max Wipfli Date: Thu, 6 Jun 2024 10:35:04 +0200 Subject: [PATCH] Allow including `Reviewed-on`/`Reviewed-by` lines for custom merge messages (#31211) This PR introduces the `ReviewedOn` and `ReviewedBy` variables for the default merge message templates (e.g., `.gitea/default_merge_message/MERGE_TEMPLATE.md`). This allows customizing the default merge messages while retaining these trailers. This also moves the associated logic out of `pull.tmpl` into the relevant Go function. This is a first contribution towards #11077. --- For illustration, this allows to recreate the "default default" merge message with the following template: ``` .gitea/default_merge_message/MERGE_TEMPLATE.md Merge pull request '${PullRequestTitle}' (${PullRequestReference}) from ${HeadBranch} into ${BaseBranch} ${ReviewedOn} ${ReviewedBy} ``` --- .../usage/merge-message-templates.en-us.md | 2 ++ services/pull/merge.go | 18 ++++++++++++++---- templates/repo/issue/view_content/pull.tmpl | 6 ++---- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/docs/content/usage/merge-message-templates.en-us.md b/docs/content/usage/merge-message-templates.en-us.md index fbdbd136f8..5116be3387 100644 --- a/docs/content/usage/merge-message-templates.en-us.md +++ b/docs/content/usage/merge-message-templates.en-us.md @@ -44,6 +44,8 @@ You can use the following variables enclosed in `${}` inside these templates whi - PullRequestIndex: Pull request's index number - PullRequestReference: Pull request's reference char with index number. i.e. #1, !2 - ClosingIssues: return a string contains all issues which will be closed by this pull request i.e. `close #1, close #2` +- ReviewedOn: Which pull request this commit belongs to. For example `Reviewed-on: https://gitea.com/foo/bar/pulls/1` +- ReviewedBy: Who approved the pull request before the merge. For example `Reviewed-by: Jane Doe ` ## Rebase diff --git a/services/pull/merge.go b/services/pull/merge.go index 20be7c5b5a..6b5e9ea330 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -46,6 +46,9 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue if err := pr.Issue.LoadPoster(ctx); err != nil { return "", "", err } + if err := pr.Issue.LoadRepo(ctx); err != nil { + return "", "", err + } isExternalTracker := pr.BaseRepo.UnitEnabled(ctx, unit.TypeExternalTracker) issueReference := "#" @@ -53,6 +56,9 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue issueReference = "!" } + reviewedOn := fmt.Sprintf("Reviewed-on: %s/%s", setting.AppURL, pr.Issue.Link()) + reviewedBy := pr.GetApprovers(ctx) + if mergeStyle != "" { templateFilepath := fmt.Sprintf(".gitea/default_merge_message/%s_TEMPLATE.md", strings.ToUpper(string(mergeStyle))) commit, err := baseGitRepo.GetBranchCommit(pr.BaseRepo.DefaultBranch) @@ -77,6 +83,8 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue "PullRequestPosterName": pr.Issue.Poster.Name, "PullRequestIndex": strconv.FormatInt(pr.Index, 10), "PullRequestReference": fmt.Sprintf("%s%d", issueReference, pr.Index), + "ReviewedOn": reviewedOn, + "ReviewedBy": reviewedBy, } if pr.HeadRepo != nil { vars["HeadRepoOwnerName"] = pr.HeadRepo.OwnerName @@ -116,20 +124,22 @@ func getMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr *issue return "", "", nil } + body = fmt.Sprintf("%s\n%s", reviewedOn, reviewedBy) + // Squash merge has a different from other styles. if mergeStyle == repo_model.MergeStyleSquash { - return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index), "", nil + return fmt.Sprintf("%s (%s%d)", pr.Issue.Title, issueReference, pr.Issue.Index), body, nil } if pr.BaseRepoID == pr.HeadRepoID { - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), "", nil + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), body, nil } if pr.HeadRepo == nil { - return fmt.Sprintf("Merge pull request '%s' (%s%d) from :%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), "", nil + return fmt.Sprintf("Merge pull request '%s' (%s%d) from :%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadBranch, pr.BaseBranch), body, nil } - return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), "", nil + return fmt.Sprintf("Merge pull request '%s' (%s%d) from %s:%s into %s", pr.Issue.Title, issueReference, pr.Issue.Index, pr.HeadRepo.FullName(), pr.HeadBranch, pr.BaseBranch), body, nil } func expandDefaultMergeMessage(template string, vars map[string]string) (message, body string) { diff --git a/templates/repo/issue/view_content/pull.tmpl b/templates/repo/issue/view_content/pull.tmpl index 77378ef1bd..69e74da3a0 100644 --- a/templates/repo/issue/view_content/pull.tmpl +++ b/templates/repo/issue/view_content/pull.tmpl @@ -199,7 +199,6 @@ {{if .AllowMerge}} {{/* user is allowed to merge */}} {{$prUnit := .Repository.MustGetUnit $.Context ctx.Consts.RepoUnitTypePullRequests}} - {{$approvers := (.Issue.PullRequest.GetApprovers ctx)}} {{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowRebaseMerge $prUnit.PullRequestsConfig.AllowSquash $prUnit.PullRequestsConfig.AllowFastForwardOnly}} {{$hasPendingPullRequestMergeTip := ""}} {{if .HasPendingPullRequestMerge}} @@ -208,11 +207,10 @@ {{end}}