Skip to content

Commit

Permalink
Add check for requiresConversationResolution (#383)
Browse files Browse the repository at this point in the history
* Add check for requiresConversationResolution

If requiresConversationResolution is set then update-branch will
attempt to merge unmergeable pull requests if there are any
unresolved conversations on the pull request. This takes this setting
into account.

* Use reviewThreads instead of comments

This only includes review comments, not regular comments on pull
requests.
  • Loading branch information
sindrig authored May 29, 2024
1 parent 4f34606 commit 77fcb7e
Show file tree
Hide file tree
Showing 9 changed files with 49 additions and 5 deletions.
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ jobs:
fetchMaxPrChecks: 100
# Optionally set the maximum amount of pull request labels to fetch (default: 10)
fetchMaxPrLabels: 10
# Optionally set the maximum amount of pull request comments to fetch (default: 50)
fetchMaxComments: 50
# The order pr checks should be fetched in. If the required checks are the last ones, consider setting to "last"
prChecksFetchOrder: first
```
Expand Down
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ inputs:
required: false
description: 'The maximum amount of pull request labels to fetch when searching for requiredLabels.'
default: '10'
fetchMaxComments:
required: false
description: 'The maximum amount of comments to fetch when checking for required conversaion resolution.'
default: '50'
prChecksFetchOrder:
required: false
description: 'The order pr checks should be fetched in. If the required checks are the last ones, consider setting to "last"'
Expand Down
19 changes: 16 additions & 3 deletions dist/index.js

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

2 changes: 1 addition & 1 deletion dist/index.js.map

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/branchProtection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ export async function getBranchProtectionRules(
nodes {
requiredApprovingReviewCount
requiredStatusCheckContexts
requiresConversationResolution
pattern
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ async function run(): Promise<void> {
prs: parseInt(core.getInput('fetchMaxPr')),
checks: parseInt(core.getInput('fetchMaxPrChecks')),
labels: parseInt(core.getInput('fetchMaxPrLabels')),
comments: parseInt(core.getInput('fetchMaxComments')),
prRunsContextOrder
}

Expand All @@ -72,6 +73,8 @@ async function run(): Promise<void> {
requiredApprovals ||
(branchProtectionRule?.requiredApprovingReviewCount ?? 0),
allRequestedReviewersMustApprove,
requiresConversationResolution:
branchProtectionRule?.requiresConversationResolution ?? false,
requiredStatusChecks: [
...requiredStatusChecks,
...(branchProtectionRule?.requiredStatusCheckContexts ?? [])
Expand Down
5 changes: 5 additions & 0 deletions src/pullRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ function getPullRequestFragment(cfg: FetchConfig): string {
name
}
}
reviewThreads(last: ${cfg.comments}) {
nodes {
isResolved
}
}
commits(last: 1) {
nodes {
commit {
Expand Down
10 changes: 10 additions & 0 deletions src/type.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {Octokit} from '@octokit/core'
export interface FetchConfig {
prs: number
labels: number
comments: number
checks: number
prRunsContextOrder: 'first' | 'last'
}
Expand All @@ -20,6 +21,7 @@ export interface Condition {
requiredApprovals: number
requiredStatusChecks: string[]
allRequestedReviewersMustApprove: boolean
requiresConversationResolution: boolean
requiredLabels: string[]
}

Expand Down Expand Up @@ -74,6 +76,9 @@ export interface PullRequestInfo {
labels: {
nodes: LabelInfo[]
}
reviewThreads: {
nodes: ReviewThread[]
}
number: number
merged: boolean
mergeable: MergeableState
Expand All @@ -89,6 +94,10 @@ export interface IssueInfo {
updatedAt?: string
}

export interface ReviewThread {
isResolved: boolean
}

export interface CommitInfo {
commit: {
statusCheckRollup?: StatusCheckRollupInfo
Expand Down Expand Up @@ -119,6 +128,7 @@ export interface BranchProtectionRuleInfo {
pattern: string
requiredApprovingReviewCount?: number
requiredStatusCheckContexts: string[]
requiresConversationResolution: boolean
}

export interface RepositoryData<T> {
Expand Down
8 changes: 7 additions & 1 deletion src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,10 @@ export function stringify<T>(obj: T): string {
return JSON.stringify(obj, null, 2)
}

function checkConversationResolution(pr: PullRequestInfo): boolean {
return pr.reviewThreads.nodes.every(v => v.isResolved)
}

// Except status check
function isSatisfyBasicConditionPr(
pr: PullRequestInfo,
Expand All @@ -40,7 +44,9 @@ function isSatisfyBasicConditionPr(
(pr.reviewRequests.totalCount === 0 ||
!condition.allRequestedReviewersMustApprove) &&
hasLabels(pr, condition) &&
minimatch(pr.baseRefName, condition.branchNamePattern ?? '*')
minimatch(pr.baseRefName, condition.branchNamePattern ?? '*') &&
(!condition.requiresConversationResolution ||
checkConversationResolution(pr))
)
}

Expand Down

0 comments on commit 77fcb7e

Please sign in to comment.