- enhance performance (remove code duplication)

- we can now fetch full reviews, no need to do reviewers individually - enhanced logging
mikepenz · Feb 26, 2023 · cad1ca5 · cad1ca5
1 parent 0ce8384
commit cad1ca5
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 37 deletions.
diff --git a/src/pullRequests.ts b/src/pullRequests.ts
@@ -47,8 +47,6 @@ type PullData = RestEndpointMethodTypes['pulls']['get']['response']['data']
 
 type PullsListData = RestEndpointMethodTypes['pulls']['list']['response']['data']
 
-type PullReviewData = RestEndpointMethodTypes['pulls']['listReviews']['response']['data']
-
 type PullReviewsData = RestEndpointMethodTypes['pulls']['listReviews']['response']['data']
 
 export class PullRequests {
@@ -143,22 +141,6 @@ export class PullRequests {
     return sortPrs(openPrs)
   }
 
-  async getReviewers(owner: string, repo: string, pr: PullRequestInfo): Promise<void> {
-    const options = this.octokit.pulls.listReviews.endpoint.merge({
-      owner,
-      repo,
-      pull_number: pr.number
-    })
-
-    for await (const response of this.octokit.paginate.iterator(options)) {
-      const reviews: PullReviewData = response.data as PullReviewData
-      pr.approvedReviewers = reviews
-        .filter(r => r.state === 'APPROVED')
-        .map(r => r.user?.login)
-        .filter(r => !!r) as string[]
-    }
-  }
-
   async getReviews(owner: string, repo: string, pr: PullRequestInfo): Promise<void> {
     const options = this.octokit.pulls.listReviews.endpoint.merge({
       owner,

diff --git a/src/releaseNotes.ts b/src/releaseNotes.ts
@@ -165,33 +165,28 @@ export class ReleaseNotes {
     })
 
     if (baseBranches.length !== 0) {
-      core.info(`ℹ️ Retrieved ${mergedPullRequests.length} PRs for ${owner}/${repo} filtered by the 'base_branches' configuration.`)
+      core.info(`ℹ️ Retrieved ${finalPrs.length} PRs for ${owner}/${repo} filtered by the 'base_branches' configuration.`)
     }
 
-    if (fetchReviewers) {
-      core.info(`ℹ️ Fetching reviewers was enabled`)
-      // update PR information with reviewers who approved
-      for (const pr of finalPrs) {
-        await pullRequestsApi.getReviewers(owner, repo, pr)
-        if (pr.approvedReviewers.length > 0) {
-          core.info(`ℹ️ Retrieved ${pr.approvedReviewers.length} reviewer(s) for PR ${owner}/${repo}/#${pr.number}`)
-        }
-      }
-    } else {
-      core.debug(`ℹ️ Fetching reviewers was disabled`)
-    }
-
-    if (fetchReviews) {
-      core.info(`ℹ️ Fetching reviews was enabled`)
+    // fetch reviewers only if enabled (requires an additional API request per PR)
+    if (fetchReviews || fetchReviewers) {
+      core.info(`ℹ️ Fetching reviews (or reviewers) was enabled`)
       // update PR information with reviewers who approved
       for (const pr of finalPrs) {
         await pullRequestsApi.getReviews(owner, repo, pr)
-        if ((pr.reviews?.length || 0) > 0) {
-          core.info(`ℹ️ Retrieved ${pr.reviews?.length || 0} review(s) for PR ${owner}/${repo}/#${pr.number}`)
+
+        const reviews = pr.reviews
+        if (reviews && (reviews?.length || 0) > 0) {
+          core.info(`ℹ️ Retrieved ${reviews.length || 0} review(s) for PR ${owner}/${repo}/#${pr.number}`)
+
+          // backwards compatiblity
+          pr.approvedReviewers = reviews.filter(r => r.state === 'APPROVED').map(r => r.author)
+        } else {
+          core.debug(`No reviewer(s) for PR ${owner}/${repo}/#${pr.number}`)
         }
       }
     } else {
-      core.debug(`ℹ️ Fetching reviews was disabled`)
+      core.debug(`ℹ️ Fetching reviews (or reviewers) was disabled`)
     }
 
     return [diffInfo, finalPrs]