Skip to content

Commit

Permalink
Preserve old behavior.
Browse files Browse the repository at this point in the history
  • Loading branch information
trasc committed Sep 12, 2024
1 parent 9c5556e commit 35981e7
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 30 deletions.
53 changes: 32 additions & 21 deletions pkg/notes/notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"net/http"
"net/url"
"regexp"
"slices"
"sort"
"strconv"
"strings"
Expand Down Expand Up @@ -1031,25 +1032,26 @@ func (g *Gatherer) prsForCommitFromSHA(sha string) (prs []*gogithub.PullRequest,
}

func (g *Gatherer) prsForCommitFromMessage(commitMessage string) (prs []*gogithub.PullRequest, err error) {
mainPrNum, err := prNumForCommitFromMessage(commitMessage)
prsNum, err := prsNumForCommitFromMessage(commitMessage)
if err != nil {
return nil, err
}

// Given the PR number that we've now converted to an integer, get the PR from
// the API
mainPr, err := g.getPr(mainPrNum)
if err != nil {
return prs, err
}

prs = append(prs, mainPr)
for _, prNum := range prsNum {
// Given the PR number that we've now converted to an integer, get the PR from
// the API
pr, err := g.getPr(prNum)
if err != nil {
return prs, err
}
prs = append(prs, pr)

originPrNum, origPrErr := originPrNumFromPr(mainPr)
if origPrErr == nil {
originPr, err := g.getPr(originPrNum)
if err == nil {
prs = append(prs, originPr)
originPrNum, origPrErr := originPrNumFromPr(pr)
if origPrErr == nil && slices.Index(prsNum, originPrNum) == -1 {
originPr, err := g.getPr(originPrNum)
if err == nil {
prs = append(prs, originPr)
}
}
}

Expand All @@ -1074,25 +1076,34 @@ var (
// stops being true, this definitely won't work anymore.
regexMergedPR = regexp.MustCompile(`Merge pull request #(?P<number>\d+)`)

// If the PR was squash merged, the regexp is different
// If the PR was squash merged, the regexp is different.
regexSquashPR = regexp.MustCompile(`\(#(?P<number>\d+)\)`)

// The branch name created by "hack/cherry_pick_pull.sh"
// The branch name created by "hack/cherry_pick_pull.sh".
regexHackBranch = regexp.MustCompile(`automated-cherry-pick-of-#(?P<number>\d+)`)

// The branch name created by k8s-infra-cherrypick-robot
// The branch name created by k8s-infra-cherrypick-robot.
regexProwBranch = regexp.MustCompile(`cherry-pick-(?P<number>\d+)-to`)
)

func prNumForCommitFromMessage(commitMessage string) (prs int, err error) {
func prsNumForCommitFromMessage(commitMessage string) (prs []int, err error) {
if pr := prForRegex(regexMergedPR, commitMessage); pr != 0 {
return pr, nil
prs = append(prs, pr)
}

if pr := prForRegex(regexSquashPR, commitMessage); pr != 0 {
return pr, nil
prs = append(prs, pr)
}

if pr := prForRegex(regexHackBranch, commitMessage); pr != 0 {
prs = append(prs, pr)
}

if len(prs) == 0 {
return nil, errNoPRIDFoundInCommitMessage
} else {
return prs, nil
}
return 0, errNoPRIDFoundInCommitMessage
}

func originPrNumFromPr(pr *gogithub.PullRequest) (origPRNum int, err error) {
Expand Down
11 changes: 8 additions & 3 deletions pkg/notes/notes_gatherer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,15 @@ func TestGatherNotes(t *testing.T) {
"when commit messages hold PR numbers, we use those and don't query to get a list of PRs for a SHA": {
commitList: []*github.RepositoryCommit{
repoCommit("123", "there is the message Merge pull request #123 somewhere in the middle"),
repoCommit("124", "and lastly in parens (#124) yeah"),
repoCommit("124", "some automated-cherry-pick-of-#124 can be found too"),
repoCommit("125", "and lastly in parens (#125) yeah"),
repoCommit("126", `all three together
some Merge pull request #126 and
another automated-cherry-pick-of-#127 with
a thing (#128) in parens`),
},
getPullRequestStubber: func(t *testing.T) getPullRequestStub {
seenPRs := newIntsRecorder(123, 124)
seenPRs := newIntsRecorder(123, 124, 125, 126, 127, 128)

return func(_ context.Context, org, repo string, prID int) (*github.PullRequest, *github.Response, error) {
checkOrgRepo(t, org, repo)
Expand All @@ -304,7 +309,7 @@ func TestGatherNotes(t *testing.T) {
return nil, nil, nil
}
},
expectedGetPullRequestCallCount: 2,
expectedGetPullRequestCallCount: 6,
},

"when the PR is a cherry pick": {
Expand Down
6 changes: 3 additions & 3 deletions pkg/notes/notes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,12 @@ func TestGetPRNumberFromCommitMessage(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
pr, err := prNumForCommitFromMessage(tc.commitMessage)
prs, err := prsNumForCommitFromMessage(tc.commitMessage)
if err != nil {
t.Fatalf("Expected no error to occur but got %v", err)
}
if pr != tc.expectedPRNumber {
t.Errorf("Expected PR number to be %d but was %d", tc.expectedPRNumber, pr)
if prs[0] != tc.expectedPRNumber {
t.Errorf("Expected PR number to be %d but was %d", tc.expectedPRNumber, prs[0])
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/notes/notes_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ func (g *Gatherer) listLeftParentCommits(opts *options.Options) ([]*commitPrPair
}

// Find and collect PR number from commit message
prNum, err := prNumForCommitFromMessage(commitPointer.Message)
prNums, err := prsNumForCommitFromMessage(commitPointer.Message)
if err == errNoPRIDFoundInCommitMessage {
logrus.WithFields(logrus.Fields{
"sha": hashString,
Expand All @@ -301,11 +301,11 @@ func (g *Gatherer) listLeftParentCommits(opts *options.Options) ([]*commitPrPair
}
logrus.WithFields(logrus.Fields{
"sha": hashString,
"pr": prNum,
"prs": prNums,
}).Debug("found PR from commit")

// Only taking the first one, assuming they are merged by Prow
pairs = append(pairs, &commitPrPair{Commit: commitPointer, PrNum: prNum})
pairs = append(pairs, &commitPrPair{Commit: commitPointer, PrNum: prNums[0]})

// Advance pointer based on left parent
hashPointer = commitPointer.ParentHashes[0]
Expand Down

0 comments on commit 35981e7

Please sign in to comment.