Skip to content

Commit

Permalink
cmd/cueckoo: add short commit hash to "Closes" for importpr
Browse files Browse the repository at this point in the history
When we import a PR from GitHub we immediately lose track of the
original commit sha because of necessary commit message rewriting.
Currently this means we have no easy way of anyone tracking exactly what
was imported.

Per a suggestion from @mvdan, we can simply add this to the "Closes"
message for now, and later consider writing a message back to the PR
from which we are importing with more information.

For #33.
  • Loading branch information
myitcv committed Mar 16, 2023
1 parent 093ad78 commit 27c1ae5
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 6 deletions.
12 changes: 9 additions & 3 deletions cmd/cueckoo/cmd/importpr.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,12 @@ func importPRDef(c *Command, args []string) error {
}
log.Printf("fetched PR into branch %q", branchName)

// Extract the commit hash
commitHash, err := run(ctx, "git", "rev-parse", "--short", "HEAD")
if err != nil {
return fmt.Errorf("failed to establish commit hash: %w", err)
}

// Set the branch upstream as the first step. If subsequent commands fail
// (they shouldn't but it can happen) we still need the upstream to have
// been set.
Expand Down Expand Up @@ -160,7 +166,7 @@ func importPRDef(c *Command, args []string) error {
if err != nil {
return err
}
msg, err = addClosesMsg(msg, prNumber)
msg, err = addClosesMsg(msg, prNumber, commitHash)
if err != nil {
return err
}
Expand Down Expand Up @@ -218,7 +224,7 @@ func run(ctx context.Context, name string, args ...string) (string, error) {
// git-interpret-trailers". If there are trailers we want to insert "Closes
// #PR as merged." as the last clear line before the trailers. If there are
// no trailers, it should be the final line in the commit message.
func addClosesMsg(msg string, pr int) (string, error) {
func addClosesMsg(msg string, pr int, commitHash string) (string, error) {
// TODO: handle carriage returns?

// Drop any trailing space. We will add back a \n at the end
Expand All @@ -240,7 +246,7 @@ func addClosesMsg(msg string, pr int) (string, error) {
msg = strings.TrimRightFunc(msg, unicode.IsSpace)

// Prepare the closes message
closes := fmt.Sprintf("Closes #%d as merged.", pr)
closes := fmt.Sprintf("Closes #%d as merged as of commit %v.", pr, commitHash)

// Add the closes message
msg += "\n\n" + closes
Expand Down
6 changes: 3 additions & 3 deletions cmd/cueckoo/cmd/importpr_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestAddCloses(t *testing.T) {
My commit message with no trailer
`,
out: "first line\n\nMy commit message with no trailer\n\nCloses #0 as merged.\n",
out: "first line\n\nMy commit message with no trailer\n\nCloses #0 as merged as of commit a01b2c3d.\n",
},
{
name: "signed-off-by trailer",
Expand All @@ -31,13 +31,13 @@ My commit message with no trailer
Signed-off-by: Paul
`,
out: "first line\n\nMy commit message with no trailer\n\nCloses #0 as merged.\n\nSigned-off-by: Paul\n",
out: "first line\n\nMy commit message with no trailer\n\nCloses #0 as merged as of commit a01b2c3d.\n\nSigned-off-by: Paul\n",
},
}

for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
got, err := addClosesMsg(c.in, c.pr)
got, err := addClosesMsg(c.in, c.pr, "a01b2c3d")
if err != nil {
t.Fatalf("got error when none expected: %v", err)
}
Expand Down

0 comments on commit 27c1ae5

Please sign in to comment.