Repository: kubernetes/release
Pull request #886 (2019-09-27 10:58:26)

Improved resolution of PR from commit SHA
I'm using this slightly version of the release-notes CLI in my projects and I was wondering if it was also be useful here.

Basically, in case the PR can't be tracked by number in the commit message, it asks github through the API to provide the PR that created a specific commit.

So this works also when the PR is not merged by the Kube bot.

k8s-ci-robot: Welcome @nicolaferraro! <br><br>It looks like this is your first PR to <a href='https://github.com/kubernetes/release'>kubernetes/release</a> 🎉. Please refer to our [pull request process documentation](https://git.k8s.io/community/contributors/guide/pull-requests.md) to help your PR have a smooth ride to approval. <br><br>You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. [Here is the bot commands documentation](https://go.k8s.io/bot-commands). <br><br>You can also check if kubernetes/release has [its own contribution guidelines](https://github.com/kubernetes/release/tree/master/CONTRIBUTING.md). <br><br>You may want to refer to our [testing guide](https://git.k8s.io/community/contributors/devel/sig-testing/testing.md) if you run into trouble with your tests not passing. <br><br>If you are having difficulty getting your pull request seen, please follow the [recommended escalation practices](https://github.com/kubernetes/community/blob/master/contributors/guide/pull-requests.md#why-is-my-pull-request-not-getting-reviewed). Also, for tips and tricks in the contribution process you may want to read the [Kubernetes contributor cheat sheet](https://git.k8s.io/community/contributors/guide/contributor-cheatsheet/README.md). We want to make sure your contribution gets all the attention it needs! <br><br>Thank you, and welcome to Kubernetes. :smiley:
k8s-ci-robot: Hi @nicolaferraro. Thanks for your PR.

I'm waiting for a [kubernetes](https://github.com/orgs/kubernetes/people) member to verify that this patch is reasonable to test. If it is, they should reply with `/ok-to-test` on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should [join the org](https://git.k8s.io/community/community-membership.md#member) to skip this step.

Once the patch is verified, the new status will be reflected by the `ok-to-test` label.

I understand the commands that are listed [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Frelease).

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>

k8s-ci-robot: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/kubernetes/release/pull/886#" title="Author self-approved">nicolaferraro</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **saschagrunert**
You can assign the PR to them by writing `/assign @saschagrunert` in a comment when ready.

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=kubernetes%2Frelease).

<details open>
Needs approval from an approver in each of these files:

- **[pkg/notes/OWNERS](https://github.com/kubernetes/release/blob/master/pkg/notes/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["saschagrunert"]} -->
saschagrunert: Hey @nicolaferraro :wave:, I think this is generally a good move forward, I just have one single question :point_up: 
justaugustus: /ok-to-test
k8s-ci-robot: @nicolaferraro: The following test **failed**, say `/retest` to rerun them all:

Test name | Commit | Details | Rerun command
--- | --- | --- | ---
pull-release-test | 3a09baa0f5b12b2596ce1501af886a60ffc3b494 | [link](https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/release/886/pull-release-test/1181800986579046400) | `/test pull-release-test`

[Full PR test history](https://prow.k8s.io/pr-history?org=kubernetes&repo=release&pr=886). [Your PR dashboard](https://gubernator.k8s.io/pr/nicolaferraro). Please help us cut down on flakes by [linking to](https://git.k8s.io/community/contributors/devel/sig-testing/flaky-tests.md#filing-issues-for-flaky-tests) an [open issue](https://github.com/kubernetes/release/issues?q=is:issue+is:open) when you hit one in your PR.

<details>

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md).  If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository. I understand the commands that are listed [here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
saschagrunert: Hey @nicolaferraro :wave: may I ask you to rebase on top of the latest master branch?
nicolaferraro: Hi, sorry, I've missed some notifications. Recovering...

Patch:
diff --git a/pkg/notes/notes.go b/pkg/notes/notes.go
index 529c5cdb..c0edaeba 100644
--- a/pkg/notes/notes.go
+++ b/pkg/notes/notes.go
@@ -455,7 +455,7 @@ func ListCommitsWithNotes(
 			"sha", commit.GetSHA(),
 		)
 
-		prs, err := PRsFromCommit(client, commit, opts...)
+		prs, err := PRsFromCommit(client, logger, commit, opts...)
 		if err != nil {
 			if err.Error() == "no matches found when parsing PR from commit" {
 				level.Debug(logger).Log(
@@ -551,27 +551,18 @@ func ListCommitsWithNotes(
 // PRsFromCommit return an API Pull Request struct given a commit struct. This is
 // useful for going from a commit log to the PR (which contains useful info such
 // as labels).
-func PRsFromCommit(client *github.Client, commit *github.RepositoryCommit, opts ...GithubApiOption) (
-	githubPRs []*github.PullRequest, err error,
+func PRsFromCommit(client *github.Client, logger log.Logger, commit *github.RepositoryCommit, opts ...GithubApiOption) (
+	[]*github.PullRequest, error,
 ) {
-	c := configFromOpts(opts...)
-
-	prs, err := prsForCommit(*commit.Commit.Message)
+	githubPRs, err := prsForCommitFromMessage(client, *commit.Commit.Message, opts...)
 	if err != nil {
-		return nil, err
-	}
-
-	for _, pr := range prs {
-		// Given the PR number that we've now converted to an integer, get the PR from
-		// the API
-		res, _, err := client.PullRequests.Get(c.ctx, c.org, c.repo, pr)
-		if err != nil {
-			return nil, err
-		}
-		githubPRs = append(githubPRs, res)
-
+		level.Debug(logger).Log(
+			"err", err,
+			"msg", "error getting the pr numbers from commit message",
+			"sha", commit.GetSHA(),
+		)
+		return prsForCommitFromSHA(client, *commit.SHA, opts...)
 	}
-
 	return githubPRs, err
 }
 
@@ -654,7 +645,62 @@ func HasString(a []string, x string) bool {
 	return false
 }
 
-func prsForCommit(commitMessage string) (prs []int, err error) {
+// prsForCommitFromSHA retrieves the PR numbers for a commit given its sha
+func prsForCommitFromSHA(client *github.Client, sha string, opts ...GithubApiOption) (prs []*github.PullRequest, err error) {
+	c := configFromOpts(opts...)
+
+	plo := &github.PullRequestListOptions{
+		State: "closed",
+		ListOptions: github.ListOptions{
+			Page:    1,
+			PerPage: 100,
+		},
+	}
+	prs, resp, err := client.PullRequests.ListPullRequestsWithCommit(c.ctx, c.org, c.repo, sha, plo)
+	if err != nil {
+		return nil, err
+	}
+
+	plo.ListOptions.Page++
+	for plo.ListOptions.Page <= resp.LastPage {
+		pResult, pResp, err := client.PullRequests.ListPullRequestsWithCommit(c.ctx, c.org, c.repo, sha, plo)
+		if err != nil {
+			return nil, err
+		}
+		prs = append(prs, pResult...)
+		resp = pResp
+		plo.ListOptions.Page++
+	}
+
+	if len(prs) == 0 {
+		return nil, errors.Errorf("no pr found for sha %s", sha)
+	}
+	return prs, nil
+}
+
+func prsForCommitFromMessage(client *github.Client, commitMessage string, opts ...GithubApiOption) (prs []*github.PullRequest, err error) {
+	c := configFromOpts(opts...)
+
+	prsNum, err := prsNumForCommitFromMessage(commitMessage)
+	if err != nil {
+		return nil, err
+	}
+
+	for _, pr := range prsNum {
+		// Given the PR number that we've now converted to an integer, get the PR from
+		// the API
+		res, _, err := client.PullRequests.Get(c.ctx, c.org, c.repo, pr)
+		if err != nil {
+			return nil, err
+		}
+		prs = append(prs, res)
+
+	}
+
+	return prs, err
+}
+
+func prsNumForCommitFromMessage(commitMessage string) (prs []int, err error) {
 	// Thankfully k8s-merge-robot commits the PR number consistently. If this ever
 	// stops being true, this definitely won't work anymore.
 	regex := regexp.MustCompile(`Merge pull request #(?P<number>\d+)`)


Test patch:
diff --git a/pkg/notes/notes_test.go b/pkg/notes/notes_test.go
index f724aaf0..713e2cb7 100644
--- a/pkg/notes/notes_test.go
+++ b/pkg/notes/notes_test.go
@@ -72,7 +72,7 @@ func TestReleaseNoteParsing(t *testing.T) {
 		fmt.Println(sha)
 		commit, _, err := client.Repositories.GetCommit(ctx, "kubernetes", "kubernetes", sha)
 		require.NoError(t, err)
-		prs, err := PRsFromCommit(client, commit)
+		prs, err := PRsFromCommit(client, nil, commit)
 		_, err = ReleaseNoteFromCommit(&Result{commit: commit, pullRequest: prs[0]}, client, "0.1")
 		require.NoError(t, err)
 	}
@@ -213,7 +213,7 @@ func TestGetPRNumberFromCommitMessage(t *testing.T) {
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			prs, err := prsForCommit(tc.commitMessage)
+			prs, err := prsNumForCommitFromMessage(tc.commitMessage)
 			if err != nil {
 				t.Fatalf("Expected no error to occur but got %v", err)
 			}