feat: improve action logs to make clear why review is dismissed (#116)

Improve logs so it's clearer why are specific reviews dismissed or
skipped.

For each review an explanation is printed out, containing commit SHAs
for which the changed files were calculated and also including a list of
changed files owned by the reviewer (directly or indirectly through team
membership):
<img width="829" alt="image"
src="https://github.com/Balvajs/dismiss-stale-reviews/assets/16807011/d60ff030-259a-4d76-a792-50e736d7e59d">

Fixes #110 and #106

Author: Balvajs

dist/main.cjs

@@ -92,37 +92,74 @@ export const calculateReviewToDismiss = async <TReview extends Review>({
     await Promise.all(
       reviews.map(async review => {
         const { author } = review
+        let isDismissed = false
 
-        debug(`Check if review of user ${author?.login} should be dismissed`)
+        console.log(
+          `Considering review from ${author?.login} and file changes between ${review.commit?.abbreviatedOid} (reviewed commit) and ${headCommit} (head commit)`,
+        )
 
         if (
           !author ||
           // if review author is mentioned directly as an owner of changed files, dismiss their review
           (author.login && changedFilesOwners.includes(`@${author.login}`))
         ) {
-          debug(
-            `User ${author?.login} is owner of changed files and their review should be dismissed`,
+          const changedFilesOwnedByReviewAuthor = filesChangedByHeadCommit
+            .filter(
+              ({ owners }) =>
+                !!owners.find(owner => owner === `@${author?.login}`),
+            )
+            .map(({ filename }) => filename)
+
+          console.log(
+            `Changed files owned by ${author?.login}:\n${changedFilesOwnedByReviewAuthor.join(
+              '\n',
+            )}`,
           )
+
           reviewsToDismiss.push(review)
+          isDismissed = true
 
           return
         }
 
         // if the files are not owned by teams we can exit early, the user is already checked
         if (!changedFilesTeamOwners.length) {
+          console.log(
+            `Review author ${author?.login} doesn't own any of changed files, nor is member of any team owning changed files.\nThe review from ${author?.login} won't be dismissed.\n`,
+          )
+
           return
         }
 
         for (const teamOwnership of changedFilesTeamOwners) {
           if (teamMembers[teamOwnership]?.includes(author.login)) {
-            debug(
-              `User ${author.login} is member of ${teamOwnership} team and their review will be dismissed`,
+            const changedFilesOwnedByAuthorsTeam = filesChangedByHeadCommit
+              .filter(
+                ({ owners }) =>
+                  !!owners.find(owner => owner === `@${teamOwnership}`),
+              )
+              .map(({ filename }) => filename)
+
+            console.log(
+              `Review author ${author?.login} is member of ${teamOwnership} team, which owns following changed files:\n${changedFilesOwnedByAuthorsTeam.join(
+                '\n',
+              )}`,
             )
+
             reviewsToDismiss.push(review)
+            isDismissed = true
           } else {
             debug(`User ${author.login} is not member of ${teamOwnership} team`)
           }
         }
+
+        if (isDismissed) {
+          console.log(`The review from ${author?.login} will be dismissed.\n`)
+        } else {
+          console.log(
+            `Review author ${author?.login} doesn't own any of changed files, nor is member of any team owning changed files.\nThe review from ${author?.login} won't be dismissed.\n`,
+          )
+        }
       }),
     )
   }

dist/main.cjs.map

@@ -57,9 +57,11 @@ export const groupReviewsByCommit = async <TReview extends Review>({
         // if commit doesn't exist, make related approve ready for dismiss and continue
         console.log(
           '\n',
-          chalk.yellow`Commit '${reviewCommit}' doesn't exist in the history. It may be because it was overwritten by force push or because it's outside of checkout depth.`,
+          chalk.yellow(
+            `Commit '${reviewCommit}' doesn't exist in the history. It may be because it was overwritten by force push or because it's outside of checkout depth.`,
+          ),
           '\n',
-          chalk.yellow`Approval by ${review.author?.login} will be removed.`,
+          chalk.yellow(`Approval by ${review.author?.login} will be removed.`),
           '\n',
         )
         reviewsWithoutHistory.push(review)

src/calculate-reviews-to-dismiss.ts

@@ -10,6 +10,20 @@ import { getInputs } from './get-inputs.ts'
 
 const chalk = new Chalk({ level: 2 })
 
+const logReviewsToDismiss = (
+  reviewsToDismiss: { author?: { login: string } | null }[],
+) => {
+  debug(`Reviews to dismiss: ${JSON.stringify(reviewsToDismiss, null, 2)}`)
+
+  console.log(
+    chalk.green(
+      `Reviews to dismiss: ${reviewsToDismiss
+        .map(({ author }) => author?.login || 'unknownLogin')
+        .join()}`,
+    ),
+  )
+}
+
 const run = async () => {
   const { ghToken, ignoreFiles, noOwnerAction, forcePushAction } = getInputs()
 
@@ -43,7 +57,7 @@ const run = async () => {
   debug(`Approving reviews: ${JSON.stringify(latestApprovedReviews, null, 2)}`)
 
   if (!latestApprovedReviews.length) {
-    console.log(chalk.green`No reviews to dismiss!`)
+    console.log(chalk.green('No reviews to dismiss!'))
 
     return
   }
@@ -57,26 +71,10 @@ const run = async () => {
       ignoreFiles,
     })
 
-    const reviewsToDismiss = reviewsToDismissContext.filesWithoutOwner
-      ? latestApprovedReviews
-      : reviewsToDismissContext.reviewsToDismiss
-
-    if (!reviewsToDismiss.length) {
-      console.log(chalk.green`No reviews to dismiss!`)
-
-      return
-    }
-
-    debug(`Reviews to dismiss: ${JSON.stringify(reviewsToDismiss, null, 2)}`)
-
-    console.log(
-      chalk.green`Reviews to dismiss: ${reviewsToDismiss
-        .map(({ author }) => author?.login || 'unknownLogin')
-        .join()}`,
-    )
-
     // if there are some files without history let the users know and dismiss reviews calculated for dismiss
     if (reviewsToDismissContext.reviewsWithoutHistory?.length) {
+      logReviewsToDismiss(reviewsToDismissContext.reviewsToDismiss)
+
       console.log(
         chalk.yellow(
           `Files diff can't be resolved for following reviews due to force push:\n${reviewsToDismissContext.reviewsWithoutHistory
@@ -114,6 +112,8 @@ const run = async () => {
     }
     // if there are any files without owner, dismiss all reviews
     else if (reviewsToDismissContext.filesWithoutOwner) {
+      logReviewsToDismiss(latestApprovedReviews)
+
       console.log(
         chalk.yellow(
           'Files without owner:\n',
@@ -149,12 +149,16 @@ const run = async () => {
           </details>
         `.replace(/  +/g, ' '),
       })
-    } else {
+    } else if (reviewsToDismissContext.reviewsToDismiss.length) {
+      logReviewsToDismiss(reviewsToDismissContext.reviewsToDismiss)
+
       await dismissReviews({
         octokit,
         reviewsToDismiss: reviewsToDismissContext.reviewsToDismiss,
         message: 'Stale reviews were dismissed based on ownership',
       })
+    } else {
+      console.log(chalk.green('No reviews to dismiss!'))
     }
   } catch (e) {
     console.error(e)