Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide Default messages for merges #9393

Merged
merged 10 commits into from
Dec 30, 2019
10 changes: 10 additions & 0 deletions custom/conf/app.ini.sample
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,16 @@ WORK_IN_PROGRESS_PREFIXES=WIP:,[WIP]
CLOSE_KEYWORDS=close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved
; List of keywords used in Pull Request comments to automatically reopen a related issue
REOPEN_KEYWORDS=reopen,reopens,reopened
; In the default merge message for squash commits include at most this many commits
DEFAULT_MERGE_MESSAGE_COMMITS_LIMIT=50
; In the default merge message for squash commits limit the size of the commit messages to this
DEFAULT_MERGE_MESSAGE_SIZE=5120
; In the default merge message for squash commits walk all commits to include all authors in the Co-authored-by otherwise just use those in the limited list
DEFAULT_MERGE_MESSAGE_ALL_AUTHORS=false
; In default merge messages limit the number of approvers listed as Reviewed-by: to this many
DEFAULT_MERGE_MESSAGE_MAX_APPROVERS=10
; In default merge messages only include approvers who are official
DEFAULT_MERGE_MESSAGE_OFFICIAL_APPROVERS_ONLY=true
Comment on lines +79 to +88
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a merge message that is provided by default when you click the appropriate merge button - the user who performs the merge has the option to change this.


[repository.issue]
; List of reasons why a Pull Request or Issue can be locked
Expand Down
5 changes: 5 additions & 0 deletions docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
keywords used in Pull Request comments to automatically close a related issue
- `REOPEN_KEYWORDS`: **reopen**, **reopens**, **reopened**: List of keywords used in Pull Request comments to automatically reopen
a related issue
- `DEFAULT_MERGE_MESSAGE_COMMITS_LIMIT`: **50**: In the default merge message for squash commits include at most this many commits. Set to `-1` to include all commits
- `DEFAULT_MERGE_MESSAGE_SIZE`: **5120**: In the default merge message for squash commits limit the size of the commit messages. Set to `-1` to have no limit.
- `DEFAULT_MERGE_MESSAGE_ALL_AUTHORS`: **false**: In the default merge message for squash commits walk all commits to include all authors in the Co-authored-by otherwise just use those in the limited list
- `DEFAULT_MERGE_MESSAGE_MAX_APPROVERS`: **10**: In default merge messages limit the number of approvers listed as `Reviewed-by:`. Set to `-1` to include all.
- `DEFAULT_MERGE_MESSAGE_OFFICIAL_APPROVERS_ONLY`: **true**: In default merge messages only include approvers who are officially allowed to review.

### Repository - Issue (`repository.issue`)

Expand Down
201 changes: 201 additions & 0 deletions models/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package models

import (
"fmt"
"io"
"strings"

"code.gitea.io/gitea/modules/git"
Expand Down Expand Up @@ -177,6 +178,206 @@ func (pr *PullRequest) GetDefaultMergeMessage() string {
return fmt.Sprintf("Merge branch '%s' of %s/%s into %s", pr.HeadBranch, pr.MustHeadUserName(), pr.HeadRepo.Name, pr.BaseBranch)
}

// GetCommitMessages returns the commit messages between head and merge base (if there is one)
func (pr *PullRequest) GetCommitMessages() string {
if err := pr.LoadIssue(); err != nil {
log.Error("Cannot load issue %d for PR id %d: Error: %v", pr.IssueID, pr.ID, err)
return ""
}

if err := pr.Issue.LoadPoster(); err != nil {
log.Error("Cannot load poster %d for pr id %d, index %d Error: %v", pr.Issue.PosterID, pr.ID, pr.Index, err)
return ""
}

if pr.HeadRepo == nil {
var err error
pr.HeadRepo, err = GetRepositoryByID(pr.HeadRepoID)
if err != nil {
log.Error("GetRepositoryById[%d]: %v", pr.HeadRepoID, err)
return ""
}
}

gitRepo, err := git.OpenRepository(pr.HeadRepo.RepoPath())
if err != nil {
log.Error("Unable to open head repository: Error: %v", err)
return ""
}
defer gitRepo.Close()

headCommit, err := gitRepo.GetBranchCommit(pr.HeadBranch)
if err != nil {
log.Error("Unable to get head commit: %s Error: %v", pr.HeadBranch, err)
return ""
}

mergeBase, err := gitRepo.GetCommit(pr.MergeBase)
if err != nil {
log.Error("Unable to get merge base commit: %s Error: %v", pr.MergeBase, err)
return ""
}

limit := setting.Repository.PullRequest.DefaultMergeMessageCommitsLimit

list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, 0)
if err != nil {
log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err)
return ""
}

maxSize := setting.Repository.PullRequest.DefaultMergeMessageSize

posterSig := pr.Issue.Poster.NewGitSig().String()

authorsMap := map[string]bool{}
authors := make([]string, 0, list.Len())
stringBuilder := strings.Builder{}
element := list.Front()
for element != nil {
commit := element.Value.(*git.Commit)

if maxSize < 0 || stringBuilder.Len() < maxSize {
toWrite := []byte(commit.CommitMessage)
if len(toWrite) > maxSize-stringBuilder.Len() && maxSize > -1 {
toWrite = append(toWrite[:maxSize-stringBuilder.Len()], "..."...)
}
if _, err := stringBuilder.Write(toWrite); err != nil {
log.Error("Unable to write commit message Error: %v", err)
return ""
}

if _, err := stringBuilder.WriteRune('\n'); err != nil {
log.Error("Unable to write commit message Error: %v", err)
return ""
}
}

authorString := commit.Author.String()
if !authorsMap[authorString] && authorString != posterSig {
authors = append(authors, authorString)
authorsMap[authorString] = true
}
element = element.Next()
}

// Consider collecting the remaining authors
if limit >= 0 && setting.Repository.PullRequest.DefaultMergeMessageAllAuthors {
techknowlogick marked this conversation as resolved.
Show resolved Hide resolved
skip := limit
limit = 30
for {
list, err := gitRepo.CommitsBetweenLimit(headCommit, mergeBase, limit, skip)
if err != nil {
log.Error("Unable to get commits between: %s %s Error: %v", pr.HeadBranch, pr.MergeBase, err)
return ""

}
if list.Len() == 0 {
break
}
element := list.Front()
for element != nil {
commit := element.Value.(*git.Commit)

authorString := commit.Author.String()
if !authorsMap[authorString] && authorString != posterSig {
authors = append(authors, authorString)
authorsMap[authorString] = true
}
element = element.Next()
}

}
}

if len(authors) > 0 {
if _, err := stringBuilder.WriteRune('\n'); err != nil {
log.Error("Unable to write to string builder Error: %v", err)
return ""
}
}

for _, author := range authors {
if _, err := stringBuilder.Write([]byte("Co-authored-by: ")); err != nil {
log.Error("Unable to write to string builder Error: %v", err)
return ""
}
if _, err := stringBuilder.Write([]byte(author)); err != nil {
log.Error("Unable to write to string builder Error: %v", err)
return ""
}
if _, err := stringBuilder.WriteRune('\n'); err != nil {
log.Error("Unable to write to string builder Error: %v", err)
return ""
}
}

return stringBuilder.String()
}

// GetApprovers returns the approvers of the pull request
func (pr *PullRequest) GetApprovers() string {

stringBuilder := strings.Builder{}
if err := pr.getReviewedByLines(&stringBuilder); err != nil {
log.Error("Unable to getReviewedByLines: Error: %v", err)
return ""
}

return stringBuilder.String()
}

func (pr *PullRequest) getReviewedByLines(writer io.Writer) error {
maxReviewers := setting.Repository.PullRequest.DefaultMergeMessageMaxApprovers

if maxReviewers == 0 {
return nil
}

sess := x.NewSession()
defer sess.Close()
if err := sess.Begin(); err != nil {
return err
}

// Note: This doesn't page as we only expect a very limited number of reviews
reviews, err := findReviews(sess, FindReviewOptions{
Type: ReviewTypeApprove,
IssueID: pr.IssueID,
OfficialOnly: setting.Repository.PullRequest.DefaultMergeMessageOfficialApproversOnly,
})
if err != nil {
log.Error("Unable to FindReviews for PR ID %d: %v", pr.ID, err)
return err
}

reviewersWritten := 0

for _, review := range reviews {
if maxReviewers > 0 && reviewersWritten > maxReviewers {
break
}

if err := review.loadReviewer(sess); err != nil && !IsErrUserNotExist(err) {
log.Error("Unable to LoadReviewer[%d] for PR ID %d : %v", review.ReviewerID, pr.ID, err)
return err
} else if review.Reviewer == nil {
continue
}
if _, err := writer.Write([]byte("Reviewed-by: ")); err != nil {
return err
}
if _, err := writer.Write([]byte(review.Reviewer.NewGitSig().String())); err != nil {
return err
}
if _, err := writer.Write([]byte{'\n'}); err != nil {
return err
}
reviewersWritten++
}
return sess.Commit()
}

// GetDefaultSquashMessage returns default message used when squash and merging pull request
func (pr *PullRequest) GetDefaultSquashMessage() string {
if err := pr.LoadIssue(); err != nil {
Expand Down
10 changes: 7 additions & 3 deletions models/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,10 @@ func GetReviewByID(id int64) (*Review, error) {

// FindReviewOptions represent possible filters to find reviews
type FindReviewOptions struct {
Type ReviewType
IssueID int64
ReviewerID int64
Type ReviewType
IssueID int64
ReviewerID int64
OfficialOnly bool
}

func (opts *FindReviewOptions) toCond() builder.Cond {
Expand All @@ -141,6 +142,9 @@ func (opts *FindReviewOptions) toCond() builder.Cond {
if opts.Type != ReviewTypeUnknown {
cond = cond.And(builder.Eq{"type": opts.Type})
}
if opts.OfficialOnly {
cond = cond.And(builder.Eq{"official": true})
}
return cond
}

Expand Down
26 changes: 25 additions & 1 deletion modules/git/repo_commit.go
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,28 @@ func (repo *Repository) FilesCountBetween(startCommitID, endCommitID string) (in

// CommitsBetween returns a list that contains commits between [last, before).
func (repo *Repository) CommitsBetween(last *Commit, before *Commit) (*list.List, error) {
stdout, err := NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
var stdout []byte
var err error
if before == nil {
stdout, err = NewCommand("rev-list", before.ID.String()).RunInDirBytes(repo.Path)
} else {
stdout, err = NewCommand("rev-list", before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
}
if err != nil {
return nil, err
}
return repo.parsePrettyFormatLogToList(bytes.TrimSpace(stdout))
}

// CommitsBetweenLimit returns a list that contains at most limit commits skipping the first skip commits between [last, before)
func (repo *Repository) CommitsBetweenLimit(last *Commit, before *Commit, limit, skip int) (*list.List, error) {
var stdout []byte
var err error
if before == nil {
stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), last.ID.String()).RunInDirBytes(repo.Path)
} else {
stdout, err = NewCommand("rev-list", "--max-count", strconv.Itoa(limit), "--skip", strconv.Itoa(skip), before.ID.String()+"..."+last.ID.String()).RunInDirBytes(repo.Path)
}
if err != nil {
return nil, err
}
Expand All @@ -328,6 +349,9 @@ func (repo *Repository) CommitsBetweenIDs(last, before string) (*list.List, erro
if err != nil {
return nil, err
}
if before == "" {
return repo.CommitsBetween(lastCommit, nil)
}
beforeCommit, err := repo.GetCommit(before)
if err != nil {
return nil, err
Expand Down
31 changes: 23 additions & 8 deletions modules/setting/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,14 @@ var (

// Pull request settings
PullRequest struct {
WorkInProgressPrefixes []string
CloseKeywords []string
ReopenKeywords []string
WorkInProgressPrefixes []string
CloseKeywords []string
ReopenKeywords []string
DefaultMergeMessageCommitsLimit int
DefaultMergeMessageSize int
DefaultMergeMessageAllAuthors bool
DefaultMergeMessageMaxApprovers int
DefaultMergeMessageOfficialApproversOnly bool
} `ini:"repository.pull-request"`

// Issue Setting
Expand Down Expand Up @@ -127,15 +132,25 @@ var (

// Pull request settings
PullRequest: struct {
WorkInProgressPrefixes []string
CloseKeywords []string
ReopenKeywords []string
WorkInProgressPrefixes []string
CloseKeywords []string
ReopenKeywords []string
DefaultMergeMessageCommitsLimit int
DefaultMergeMessageSize int
DefaultMergeMessageAllAuthors bool
DefaultMergeMessageMaxApprovers int
DefaultMergeMessageOfficialApproversOnly bool
}{
WorkInProgressPrefixes: []string{"WIP:", "[WIP]"},
// Same as GitHub. See
// https://help.github.com/articles/closing-issues-via-commit-messages
CloseKeywords: strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","),
ReopenKeywords: strings.Split("reopen,reopens,reopened", ","),
CloseKeywords: strings.Split("close,closes,closed,fix,fixes,fixed,resolve,resolves,resolved", ","),
ReopenKeywords: strings.Split("reopen,reopens,reopened", ","),
DefaultMergeMessageCommitsLimit: 50,
DefaultMergeMessageSize: 5 * 1024,
DefaultMergeMessageAllAuthors: false,
DefaultMergeMessageMaxApprovers: 10,
DefaultMergeMessageOfficialApproversOnly: true,
},

// Issue settings
Expand Down
8 changes: 5 additions & 3 deletions templates/repo/issue/view_content/pull.tmpl
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
{{end}}
{{if .AllowMerge}}
{{$prUnit := .Repository.MustGetUnit $.UnitTypePullRequests}}
{{$approvers := .Issue.PullRequest.GetApprovers}}
{{if or $prUnit.PullRequestsConfig.AllowMerge $prUnit.PullRequestsConfig.AllowRebase $prUnit.PullRequestsConfig.AllowRebaseMerge $prUnit.PullRequestsConfig.AllowSquash}}
<div class="ui divider"></div>
{{if $prUnit.PullRequestsConfig.AllowMerge}}
Expand All @@ -141,7 +142,7 @@
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
</div>
<div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$approvers}}</textarea>
</div>
<button class="ui green button" type="submit" name="do" value="merge">
{{$.i18n.Tr "repo.pulls.merge_pull_request"}}
Expand Down Expand Up @@ -173,7 +174,7 @@
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultMergeMessage}}">
</div>
<div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$approvers}}</textarea>
</div>
<button class="ui green button" type="submit" name="do" value="rebase-merge">
{{$.i18n.Tr "repo.pulls.rebase_merge_commit_pull_request"}}
Expand All @@ -185,14 +186,15 @@
</div>
{{end}}
{{if $prUnit.PullRequestsConfig.AllowSquash}}
{{$commitMessages := .Issue.PullRequest.GetCommitMessages}}
<div class="ui form squash-fields" style="display: none">
<form action="{{.Link}}/merge" method="post">
{{.CsrfTokenHtml}}
<div class="field">
<input type="text" name="merge_title_field" value="{{.Issue.PullRequest.GetDefaultSquashMessage}}">
</div>
<div class="field">
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}"></textarea>
<textarea name="merge_message_field" rows="5" placeholder="{{$.i18n.Tr "repo.editor.commit_message_desc"}}">{{$commitMessages}}{{$approvers}}</textarea>
</div>
<button class="ui green button" type="submit" name="do" value="squash">
{{$.i18n.Tr "repo.pulls.squash_merge_pull_request"}}
Expand Down