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 11, 2024
1 parent 7e22c05 commit caa1527
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 28 deletions.
47 changes: 28 additions & 19 deletions pkg/notes/notes.go
Original file line number Diff line number Diff line change
Expand Up @@ -1020,28 +1020,28 @@ 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
mainPr, err := g.getPr(prNum)
if err != nil {
return prs, err
}
prs = append(prs, mainPr)

originPrNum, origPrErr := originPrNumFromPr(mainPr)
if origPrErr == nil {
originPr, err := g.getPr(originPrNum)
if err == nil {
prs = append(prs, originPr)
originPrNum, origPrErr := originPrNumFromPr(mainPr)
if origPrErr == nil {
originPr, err := g.getPr(originPrNum)
if err == nil {
prs = append(prs, originPr)
}
}
}

return prs, err
}

Expand Down Expand Up @@ -1073,15 +1073,24 @@ var (
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 {

Check failure on line 1077 in pkg/notes/notes.go

View workflow job for this annotation

GitHub Actions / lint

Comment should end in a period (godot)
return pr, nil
prs = append(prs, pr)
}

Check failure on line 1080 in pkg/notes/notes.go

View workflow job for this annotation

GitHub Actions / lint

Comment should end in a period (godot)
if pr := prForRegex(regexSquashPR, commitMessage); pr != 0 {
return pr, nil
prs = append(prs, pr)
}

Check failure on line 1083 in pkg/notes/notes.go

View workflow job for this annotation

GitHub Actions / lint

Comment should end in a period (godot)

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 @@ -289,10 +289,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 @@ -302,7 +307,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 @@ -222,12 +222,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 @@ -278,7 +278,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 @@ -299,11 +299,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 caa1527

Please sign in to comment.