mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-02-18 19:57:49 -05:00
fix(ui)!: remove squash merge committer trailer admin option (#11096)
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). Closes: #11097 Closes: Codeberg/Community#2030 Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/11096 Reviewed-by: 0ko <0ko@noreply.codeberg.org> Reviewed-by: Gusted <gusted@noreply.codeberg.org> Co-authored-by: Robert Wolff <mahlzahn@posteo.de> Co-committed-by: Robert Wolff <mahlzahn@posteo.de>
This commit is contained in:
Robert Wolff
0ko
parent
db037fca50
commit
70865730e6
6 changed files with 1 additions and 72 deletions
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
},
|
||||
|
||||
|
|
|
|||
1
release-notes/11096.md
Normal file
1
release-notes/11096.md
Normal file
|
|
@ -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).
|
||||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -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"))
|
||||
|
|
|
|||
Loading…
Reference in a new issue