Skip to content

Commit

Permalink
feat: stop treating same head and review SHA as force (#300)
Browse files Browse the repository at this point in the history
After some testing, I noticed that when the base branch is merged/PR is
rebased **cleanly**, Github API changes the associated SHA of PR review
to the latest commit. However, if merge/rebase is **not clean**, and
there are some changes, the associated SHA stays the same.
That means that GitHub is completely responsible for correctly
identifying whether there were some changes since the last review, and
this action can't distinguish between a clean merge/force push and a
re-run of the action.

So to prevent unexpected review removals when the base branch is
merged/PR is rebased **cleanly**, this PR removes "PR review dismiss"
for `reviewAssociatedCommit === headCommit` condition.
</br>

BREAKING CHANGE: When `head` SHA and SHA associated with the review are
the same, it is no longer interpreted as force push, and the review is
**not** dismissed.
If the `head` and review SHAs are the same, it's either because there
were no new commits, or because the base branch was merged/PR was
rebased **cleanly**.

Fixes #253
  • Loading branch information
Balvajs authored Jan 28, 2024
1 parent 1eca1e0 commit 657f387
Show file tree
Hide file tree
Showing 5 changed files with 6 additions and 11 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ jobs:
app_id: ${{ secrets.MANAGE_REVIEWS_BOT_ID }}
private_key: ${{ secrets.MANAGE_REVIEWS_BOT_PEM }}

- uses: balvajs/dismiss-stale-reviews@v1
- uses: balvajs/dismiss-stale-reviews@v3
with:
token: ${{ steps.get-token.outputs.token }}
```
Expand Down
5 changes: 1 addition & 4 deletions dist/main.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/main.cjs.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lint-staged.config.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default {
'*.{js,ts,json,yml,md,mdx}': filenames =>
`prettier --write ${filenames.join(' ')}`,
'*.{js,ts,json}': () => 'pnpm run package',
'*.{js,ts,json}': () => ['pnpm run package', 'git add dist'],
}
6 changes: 2 additions & 4 deletions src/calculate-reviews-to-dismiss.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,13 +98,11 @@ export const calculateReviewToDismiss = async <TReview extends Review>({
`Considering review from ${author?.login} and file changes between ${review.commit?.oid} (reviewed commit) and ${headCommit} (head commit)`,
)

// in case there is no diff because head and review commit matches, skip that review
if (review.commit?.oid === headCommit) {
console.log(
'The review commit sha is the same as head commit sha and changed files can’t be resolved. This is caused by force-push.',
'The review commit sha is the same as head commit sha. That means that there were no changes since the review, or the base branch was merged/rebased cleanly.',
)
isDismissed = true
reviewsWithoutHistory.push(review)
reviewsToDismiss.push(review)
} else if (
!author ||
// if review author is mentioned directly as an owner of changed files, dismiss their review
Expand Down

0 comments on commit 657f387

Please sign in to comment.