Skip to content

Commit

Permalink
mr create: Use remote source branch for commit logs
Browse files Browse the repository at this point in the history
As reported in issue 635, in some circumstances the 'mr create' command
does not show the correct information when calculating the number of
commits or generating the commit log.

This happens because the commit comparison is done using the local
source branch which may have different content than the remote source
branch.

Use the remote source branch to calculate the number of commits and the
commit log changelog.

Signed-off-by: Prarit Bhargava <[email protected]>
  • Loading branch information
prarit committed Mar 19, 2021
1 parent 4f8cc17 commit 610a795
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 9 deletions.
7 changes: 4 additions & 3 deletions cmd/mr_create.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,17 +285,18 @@ func mrText(base, head, sourceRemote, targetRemote string, coverLetterFormat boo
err error
)
remoteBase := fmt.Sprintf("%s/%s", targetRemote, base)
targetBase := fmt.Sprintf("%s/%s", sourceRemote, head)
commitMsg = ""

numCommits := git.NumberCommits(remoteBase, head)
numCommits := git.NumberCommits(remoteBase, targetBase)
if numCommits == 1 {
commitMsg, err = git.LastCommitMessage()
if err != nil {
return "", err
}
}
if numCommits == 0 {
return "", errors.New("Aborting: Resulting Merge Request would have had 0 commits.")
return "", fmt.Errorf("Aborting: The resulting Merge Request from %s to %s has 0 commits.", remoteBase, targetBase)
}

const tmpl = `{{if .InitMsg}}{{.InitMsg}}{{end}}
Expand All @@ -312,7 +313,7 @@ func mrText(base, head, sourceRemote, targetRemote string, coverLetterFormat boo

mrTmpl := lab.LoadGitLabTmpl(lab.TmplMR)

commitLogs, err := git.Log(remoteBase, head)
commitLogs, err := git.Log(remoteBase, targetBase)
if err != nil {
return "", err
}
Expand Down
8 changes: 4 additions & 4 deletions cmd/mr_create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@ import (
// MR Create is tested in cmd/mr_test.go

func Test_mrText(t *testing.T) {
text, err := mrText("master", "mrtest", "lab-testing", "origin", false)
text, err := mrText("master", "mrtest", "origin", "origin", false)
if err != nil {
t.Log(text)
t.Fatal(err)
}
require.Contains(t, text, `
I am the default merge request template for lab
# Requesting a merge into origin:master from lab-testing:mrtest (12 commits)
# Requesting a merge into origin:master from origin:mrtest (12 commits)
#
# Write a message for this merge request. The first block
# of text is the title and the rest is the description.
Expand All @@ -29,15 +29,15 @@ I am the default merge request template for lab
}

func Test_mrText_CoverLetter(t *testing.T) {
coverLetter, err := mrText("master", "mrtest", "lab-testing", "origin", true)
coverLetter, err := mrText("master", "mrtest", "origin", "origin", true)
if err != nil {
t.Log(coverLetter)
t.Fatal(err)
}
require.Contains(t, coverLetter, `
I am the default merge request template for lab
# Requesting a merge into origin:master from lab-testing:mrtest (12 commits)
# Requesting a merge into origin:master from origin:mrtest (12 commits)
#
# Write a message for this merge request. The first block
# of text is the title and the rest is the description.
Expand Down
4 changes: 2 additions & 2 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,8 +368,8 @@ func NumberCommits(sha1, sha2 string) int {
cmd.Stderr = nil
CmdOut, err := cmd.Output()
if err != nil {
fmt.Printf("There are no commits between %s and %s\n", sha1, sha2)
log.Fatal(err)
// silently fail and handle the return of 0 at caller
return 0
}
numLines := strings.Count(string(CmdOut), "\n")
return numLines
Expand Down

0 comments on commit 610a795

Please sign in to comment.