diff --git a/custom/conf/app.example.ini b/custom/conf/app.example.ini index 568091b058..b7aa3571df 100644 --- a/custom/conf/app.example.ini +++ b/custom/conf/app.example.ini @@ -1132,9 +1132,6 @@ LEVEL = Info ;; If an squash commit's comment should be populated with the commit messages of the squashed commits ;POPULATE_SQUASH_COMMENT_WITH_COMMIT_MESSAGES = false ;; -;; Add co-authored-by and co-committed-by trailers if committer does not match author -;ADD_CO_COMMITTER_TRAILERS = true -;; ;; Retarget child pull requests to the parent pull request branch target on merge of parent pull request. It only works on merged PRs where the head and base branch target the same repo. ;RETARGET_CHILDREN_ON_MERGE = true diff --git a/modules/setting/repository.go b/modules/setting/repository.go index 7e774f0139..001032e3cb 100644 --- a/modules/setting/repository.go +++ b/modules/setting/repository.go @@ -96,7 +96,6 @@ var ( DefaultMergeMessageOfficialApproversOnly bool DefaultUpdateStyle string PopulateSquashCommentWithCommitMessages bool - AddCoCommitterTrailers bool RetargetChildrenOnMerge bool } `ini:"repository.pull-request"` @@ -227,7 +226,6 @@ var ( DefaultMergeMessageOfficialApproversOnly bool DefaultUpdateStyle string PopulateSquashCommentWithCommitMessages bool - AddCoCommitterTrailers bool RetargetChildrenOnMerge bool }{ WorkInProgressPrefixes: []string{"WIP:", "[WIP]"}, @@ -243,7 +241,6 @@ var ( DefaultMergeMessageOfficialApproversOnly: true, DefaultUpdateStyle: "merge", PopulateSquashCommentWithCommitMessages: false, - AddCoCommitterTrailers: true, RetargetChildrenOnMerge: true, }, diff --git a/release-notes/11096.md b/release-notes/11096.md new file mode 100644 index 0000000000..6db7331bd4 --- /dev/null +++ b/release-notes/11096.md @@ -0,0 +1 @@ +fix(ui)!: Remove the instance configuration option `repository.pull-request.ADD_CO_COMMITTER_TRAILERS` (was enabled by default). It was responsible for addition of unexpected trailers to commit messages in squash merges. These trailers were `Co-authored-by: ` and `Co-committed-by: `. Both used the pull request author as value, who is also assigned as the author of the squash merge commit, which they were just repeating. Furthermore, `Co-committed-by: ` is an uncommon commit trailer, and there is only one committer for a commit. The trailers were being added by Forgejo while performing the merge, bypassing user input in the UI and weren't shown in it. See further description and more examples in [#11097](https://codeberg.org/forgejo/forgejo/issues/11097). diff --git a/services/pull/merge.go b/services/pull/merge.go index 0977623bfb..1bcd11406c 100644 --- a/services/pull/merge.go +++ b/services/pull/merge.go @@ -208,41 +208,6 @@ func GetDefaultMergeMessage(ctx context.Context, baseGitRepo *git.Repository, pr return getMergeMessage(ctx, baseGitRepo, pr, mergeStyle, nil) } -func AddCommitMessageTrailer(message, tailerKey, tailerValue string) string { - trailerLine := tailerKey + ": " + tailerValue - message = strings.ReplaceAll(message, "\r\n", "\n") - message = strings.ReplaceAll(message, "\r", "\n") - if strings.Contains(message, "\n"+trailerLine+"\n") || strings.HasSuffix(message, "\n"+trailerLine) { - return message - } - - if !strings.HasSuffix(message, "\n") { - message += "\n" - } - lastNewLine := strings.LastIndexByte(message[:len(message)-1], '\n') - keyEnd := -1 - if lastNewLine != -1 { - keyEnd = strings.IndexByte(message[lastNewLine:], ':') - if keyEnd != -1 { - keyEnd += lastNewLine - } - } - var lastLineKey string - if lastNewLine != -1 && keyEnd != -1 { - lastLineKey = message[lastNewLine+1 : keyEnd] - } - - isLikelyTrailerLine := lastLineKey != "" && unicode.IsUpper(rune(lastLineKey[0])) && strings.Contains(message, "-") - for i := 0; isLikelyTrailerLine && i < len(lastLineKey); i++ { - r := rune(lastLineKey[i]) - isLikelyTrailerLine = unicode.IsLetter(r) || unicode.IsDigit(r) || r == '-' - } - if !strings.HasSuffix(message, "\n\n") && !isLikelyTrailerLine { - message += "\n" - } - return message + trailerLine -} - // Merge merges pull request to base repository. // Caller should check PR is ready to be merged (review and status checks) func Merge(ctx context.Context, pr *issues_model.PullRequest, doer *user_model.User, baseGitRepo *git.Repository, mergeStyle repo_model.MergeStyle, expectedHeadCommitID, message string, wasAutoMerged bool) error { diff --git a/services/pull/merge_squash.go b/services/pull/merge_squash.go index 898f4c977d..715dc7d4cb 100644 --- a/services/pull/merge_squash.go +++ b/services/pull/merge_squash.go @@ -11,7 +11,6 @@ import ( "forgejo.org/modules/container" "forgejo.org/modules/git" "forgejo.org/modules/log" - "forgejo.org/modules/setting" ) // doMergeStyleSquash gets a commit author signature for squash commits @@ -63,11 +62,6 @@ func doMergeStyleSquash(ctx *mergeContext, message string) error { return err } - if setting.Repository.PullRequest.AddCoCommitterTrailers && ctx.committer.String() != sig.String() { - // add trailer - message = AddCommitMessageTrailer(message, "Co-authored-by", sig.String()) - message = AddCommitMessageTrailer(message, "Co-committed-by", sig.String()) // FIXME: this one should be removed, it is not really used or widely used - } cmdCommit := git.NewCommand(ctx, "commit"). AddOptionFormat("--author='%s <%s>'", sig.Name, sig.Email). AddOptionFormat("--message=%s", message) diff --git a/services/pull/merge_test.go b/services/pull/merge_test.go index 0766a46cea..62d6604c98 100644 --- a/services/pull/merge_test.go +++ b/services/pull/merge_test.go @@ -138,31 +138,6 @@ func Test_expandDefaultMergeMessage(t *testing.T) { } } -func TestAddCommitMessageTailer(t *testing.T) { - // add tailer for empty message - assert.Equal(t, "\n\nTest-tailer: TestValue", AddCommitMessageTrailer("", "Test-tailer", "TestValue")) - - // add tailer for message without newlines - assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title", "Test-tailer", "TestValue")) - assert.Equal(t, "title\n\nNot tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNot tailer: xxx", "Test-tailer", "TestValue")) - assert.Equal(t, "title\n\nNotTailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nNotTailer: xxx", "Test-tailer", "TestValue")) - assert.Equal(t, "title\n\nnot-tailer: xxx\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nnot-tailer: xxx", "Test-tailer", "TestValue")) - - // add tailer for message with one EOL - assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n", "Test-tailer", "TestValue")) - - // add tailer for message with two EOLs - assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\n", "Test-tailer", "TestValue")) - - // add tailer for message with existing tailer (won't duplicate) - assert.Equal(t, "title\n\nTest-tailer: TestValue", AddCommitMessageTrailer("title\n\nTest-tailer: TestValue", "Test-tailer", "TestValue")) - assert.Equal(t, "title\n\nTest-tailer: TestValue\n", AddCommitMessageTrailer("title\n\nTest-tailer: TestValue\n", "Test-tailer", "TestValue")) - - // add tailer for message with existing tailer and different value (will append) - assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTrailer("title\n\nTest-tailer: v1", "Test-tailer", "v2")) - assert.Equal(t, "title\n\nTest-tailer: v1\nTest-tailer: v2", AddCommitMessageTrailer("title\n\nTest-tailer: v1\n", "Test-tailer", "v2")) -} - func prepareLoadMergeMessageTemplates(targetDir string) error { for _, template := range []string{"MERGE", "REBASE", "REBASE-MERGE", "SQUASH", "MANUALLY-MERGED", "REBASE-UPDATE-ONLY"} { file, err := os.Create(path.Join(targetDir, template+"_TEMPLATE.md"))