Repositories

The "git appraise submit" command supports a "--merge" option, that forces the creation of a merge commit for the review (sometimes called a "merge bubble").

When this is done, an editor is launched for the user to provide the message for that merge commit. Previously, the editor was being populated with the default message which has the form "merge branch ... with ...". This change replaces that with a more appropriate message that specifies the review being submitted, and the description of that review.

This is important in that a user may want to use merge commits to allow themselves to both maintain a full history of their changes (including changes made in response to review feedback) and have the ability to list only the high level changes corresponding to the submission of each review.

More specifically, if a user wants to always submit their reviews with the --merge option, then this change makes it so that they can use the command "git log --first-parent" to view only the portion of their history that corresponds to review submissions, while still having the displayed messages provide a meaningful summary of the change.

Modified commands/submit.go

@@ -75,7 +75,8 @@ func submitReview(repo repository.Repo, args []string) error {
return err
}
if *submitMerge {
return repo.MergeRef(source, false)
submitMessage := fmt.Sprintf("Submitting review %.12s", r.Revision)
return repo.MergeRef(source, false, submitMessage, r.Request.Description)
} else if *submitRebase {
return repo.RebaseRef(source)
} else {

Modified repository/git.go

@@ -244,13 +244,19 @@ func (repo *GitRepo) SwitchToRef(ref string) error {
//
// The ref argument is the ref to merge, and fastForward indicates that the
// current ref should only move forward, as opposed to creating a bubble merge.
func (repo *GitRepo) MergeRef(ref string, fastForward bool) error {
// The messages argument(s) provide text that should be included in the default
// merge commit message (separated by blank lines).
func (repo *GitRepo) MergeRef(ref string, fastForward bool, messages ...string) error {
args := []string{"merge"}
if fastForward {
args = append(args, "--ff", "--ff-only")
} else {
args = append(args, "--no-ff")
}
if len(messages) > 0 {
commitMessage := strings.Join(messages, "\n\n")
args = append(args, "-e", "-m", commitMessage)
}
args = append(args, ref)
return repo.runGitCommandInline(args...)
}

Modified repository/mock_repo.go

@@ -345,7 +345,7 @@ func (r mockRepoForTest) SwitchToRef(ref string) error {
//
// The ref argument is the ref to merge, and fastForward indicates that the
// current ref should only move forward, as opposed to creating a bubble merge.
func (r mockRepoForTest) MergeRef(ref string, fastForward bool) error { return nil }
func (r mockRepoForTest) MergeRef(ref string, fastForward bool, messages ...string) error { return nil }
// RebaseRef rebases the given ref into the current one.
func (r mockRepoForTest) RebaseRef(ref string) error { return nil }

Modified repository/repo.go

@@ -97,7 +97,9 @@ type Repo interface {
//
// The ref argument is the ref to merge, and fastForward indicates that the
// current ref should only move forward, as opposed to creating a bubble merge.
MergeRef(ref string, fastForward bool) error
// The messages argument(s) provide text that should be included in the default
// merge commit message (separated by blank lines).
MergeRef(ref string, fastForward bool, messages ...string) error
// RebaseRef rebases the given ref into the current one.
RebaseRef(ref string) error