Skip to content

Commit

Permalink
fix: change order of edge case resolution (#115)
Browse files Browse the repository at this point in the history
The `no-owner` edge case was resolved before `force-push` edge case.
That doesn't make sense, because when there is a `force-push`, then the
history can't be resolved and it can't be determined what changed and
what didn't.
This PR gives the `force-push` case precedence, so it always resolves
first.

Fixes the #95
  • Loading branch information
Balvajs authored Oct 5, 2023
1 parent 358949f commit 7895e47
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 64 deletions.
56 changes: 28 additions & 28 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 src/get-inputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getInput, getMultilineInput } from '@actions/core'

function isValidDismissActionInput(
dismissAction: string,
): dismissAction is 'dismissAction' | 'dismiss-none' {
): dismissAction is 'dismiss-all' | 'dismiss-none' {
return dismissAction === 'dismiss-all' || dismissAction === 'dismiss-none'
}

Expand Down
69 changes: 35 additions & 34 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,20 @@ const run = async () => {
.join()}`,
)

// if there are any files without owner, dismiss all reviews
if (reviewsToDismissContext.filesWithoutOwner) {
// if there are some files without history let the users know and dismiss reviews calculated for dismiss
if (reviewsToDismissContext.reviewsWithoutHistory?.length) {
console.log(
chalk.yellow(
'Files without owner:\n',
reviewsToDismissContext.filesWithoutOwner.join('/n'),
`Files diff can't be resolved for following reviews due to force push:\n${reviewsToDismissContext.reviewsWithoutHistory
.map(({ author }) => author?.login)
.join('\n')}\n`,
),
)

if (noOwnerAction === 'dismiss-none') {
if (forcePushAction === 'dismiss-none') {
console.log(
chalk.yellow(
'"no-owner-action" is set to "dismiss-none", so no reviews are dismissed.',
'"force-push-action" is set to "dismiss-none", so no reviews are dismissed.',
),
)

Expand All @@ -96,36 +97,34 @@ const run = async () => {

await dismissReviews({
octokit,
reviewsToDismiss: latestApprovedReviews,
reviewsToDismiss: reviewsToDismissContext.reviewsToDismiss,
message: `
<details>
<summary>Because some files don’t have owner, all reviews are dismissed.</summary>
<p>
If you know who should own following files, consider adding the owner to \`.github/CODEOWNERS\` file.
- \`${reviewsToDismissContext.filesWithoutOwner
.join('`\n- `')
.replace(/_/g, '&#95;')}\`
</p>
</details>
`.replace(/ +/g, ' '),
<details>
<summary>Following reviews were removed because related commit was overwritten by force push.</summary>
<p>
- \`${reviewsToDismissContext.reviewsWithoutHistory
.map(({ author }) => author?.login)
.join('`\n- `')}\`
</p>
</details>
`.replace(/ +/g, ' '),
})
// if there are some files without history let the users know and dismiss reviews calculated for dismiss
} else if (reviewsToDismissContext.reviewsWithoutHistory.length) {
}
// if there are any files without owner, dismiss all reviews
else if (reviewsToDismissContext.filesWithoutOwner) {
console.log(
chalk.yellow(
`Files diff can't be resolved for following reviews due to force push:\n${reviewsToDismissContext.reviewsWithoutHistory
.map(({ author }) => author?.login)
.join('\n')}\n`,
'Files without owner:\n',
reviewsToDismissContext.filesWithoutOwner.join('/n'),
),
)

if (forcePushAction === 'dismiss-none') {
if (noOwnerAction === 'dismiss-none') {
console.log(
chalk.yellow(
'"force-push-action" is set to "dismiss-none", so no reviews are dismissed.',
'"no-owner-action" is set to "dismiss-none", so no reviews are dismissed.',
),
)

Expand All @@ -134,16 +133,18 @@ const run = async () => {

await dismissReviews({
octokit,
reviewsToDismiss: reviewsToDismissContext.reviewsToDismiss,
reviewsToDismiss: latestApprovedReviews,
message: `
<details>
<summary>Following reviews were removed because related commit was overwritten by force push.</summary>
<summary>Because some files don’t have owner, all reviews are dismissed.</summary>
<p>
- \`${reviewsToDismissContext.reviewsWithoutHistory
.map(({ author }) => author?.login)
.join('`\n- `')}\`
If you know who should own following files, consider adding the owner to \`.github/CODEOWNERS\` file.
- \`${reviewsToDismissContext.filesWithoutOwner
.join('`\n- `')
.replace(/_/g, '&#95;')}\`
</p>
</details>
`.replace(/ +/g, ' '),
Expand Down

0 comments on commit 7895e47

Please sign in to comment.