Skip to content

Commit

Permalink
add: optimistic_updates config to prioritize updates over status chec…
Browse files Browse the repository at this point in the history
…ks (#64)

instead of waiting for status checks to finish before updating we will update the branch.
  • Loading branch information
chdsbd authored and kodiakhq[bot] committed Jul 6, 2019
1 parent c32b8a5 commit 229e8a9
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 8 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ Kodiak won't merge PRs if branch protection is disabled.
delete_branch_on_merge = true # default: false
block_on_reviews_requested = false # default: false
notify_on_conflict = true # default: true
optimistic_updates = true # default: true

[merge.message]
title = "pull_request_title" # default: "github_default"
Expand Down
2 changes: 2 additions & 0 deletions kodiak/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ class Merge(BaseModel):
block_on_reviews_requested: bool = False
# comment on merge conflict and remove automerge label
notify_on_conflict: bool = True
# don't wait for status checks to run before updating branch
optimistic_updates: bool = True
# configuration for commit message of merge
message: MergeMessage = MergeMessage()

Expand Down
33 changes: 25 additions & 8 deletions kodiak/evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,11 @@ def mergeable(
if successful_reviews < branch_protection.requiredApprovingReviewCount:
raise NotQueueable("missing required review count")

if branch_protection.requiresCommitSignatures and not valid_signature:
raise NotQueueable("missing required signature")

required: typing.Set[str] = set()
passing: typing.Set[str] = set()
if branch_protection.requiresStatusChecks:
failing_contexts: typing.List[str] = []
pending_contexts: typing.List[str] = []
Expand Down Expand Up @@ -241,17 +246,29 @@ def mergeable(
# is a similar question for the review counting.
raise NotQueueable("failing required status checks")
passing = set(passing_contexts)
if len(required - passing) > 0:
raise WaitingForChecks("missing required status checks")

if branch_protection.requiresCommitSignatures:
if not valid_signature:
raise NotQueueable("missing required signature")
if (
need_branch_update = (
branch_protection.requiresStrictStatusChecks
and pull_request.mergeStateStatus == MergeStateStatus.BEHIND
):
raise NeedsBranchUpdate("behind branch. need update")
)
wait_for_checks = (
branch_protection.requiresStatusChecks and len(required - passing) > 0
)

# prioritize branch updates over waiting for status checks to complete
if config.merge.optimistic_updates:
if need_branch_update:
raise NeedsBranchUpdate("behind branch. need update")
if wait_for_checks:
raise WaitingForChecks("missing required status checks")
# almost the same as the pervious case, but we prioritize status checks
# over branch updates.
else:
if wait_for_checks:
raise WaitingForChecks("missing required status checks")
if need_branch_update:
raise NeedsBranchUpdate("behind branch. need update")

raise NotQueueable("Could not determine why PR is blocked")

# okay to merge
Expand Down
1 change: 1 addition & 0 deletions kodiak/test/fixtures/config/v1-default.toml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ method = "merge"
delete_branch_on_merge = false
block_on_reviews_requested = false
notify_on_conflict = true
optimistic_updates = true

[merge.message]
title = "github_default"
Expand Down
1 change: 1 addition & 0 deletions kodiak/test/fixtures/config/v1-opposite.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ method = "squash" # default: "merge"
delete_branch_on_merge = true # default: False
block_on_reviews_requested = true # default: false
notify_on_conflict = false # default: true
optimistic_updates = false # default: true

[merge.message]
title = "pull_request_title" # default: "github_default"
Expand Down
1 change: 1 addition & 0 deletions kodiak/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ def test_config_parsing_opposite() -> None:
delete_branch_on_merge=True,
block_on_reviews_requested=True,
notify_on_conflict=False,
optimistic_updates=False,
message=MergeMessage(
title=MergeTitleStyle.pull_request_title,
body=MergeBodyStyle.pull_request_body,
Expand Down
49 changes: 49 additions & 0 deletions kodiak/test_evaluation.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from datetime import datetime, timedelta
from typing import List

import pytest

Expand Down Expand Up @@ -817,3 +818,51 @@ def test_app_id(
valid_signature=False,
valid_merge_methods=[],
)


def test_config_merge_optimistic_updates(
pull_request: PullRequest, config: V1, branch_protection: BranchProtectionRule
) -> None:
"""
If optimisitc_updates are enabled, branch updates should be prioritized over
waiting for running status checks to complete.
Otherwise, status checks should be checked before updating.
"""
branch_protection.requiredApprovingReviewCount = 0

branch_protection.requiresStrictStatusChecks = True
pull_request.mergeStateStatus = MergeStateStatus.BEHIND

branch_protection.requiresStatusChecks = True
branch_protection.requiredStatusCheckContexts = ["ci/lint", "ci/test"]
contexts: List[StatusContext] = []

config.merge.optimistic_updates = True
with pytest.raises(NeedsBranchUpdate):
mergeable(
app_id="1234",
config=config,
pull_request=pull_request,
branch_protection=branch_protection,
review_requests_count=0,
reviews=[],
contexts=contexts,
check_runs=[],
valid_signature=False,
valid_merge_methods=[MergeMethod.squash],
)
config.merge.optimistic_updates = False
with pytest.raises(WaitingForChecks):
mergeable(
app_id="1234",
config=config,
pull_request=pull_request,
branch_protection=branch_protection,
review_requests_count=0,
reviews=[],
contexts=contexts,
check_runs=[],
valid_signature=False,
valid_merge_methods=[MergeMethod.squash],
)

0 comments on commit 229e8a9

Please sign in to comment.