Skip to content

Commit

Permalink
Add option to hide status message when no automerge label (#757)
Browse files Browse the repository at this point in the history
- Add option to hide status message on PR's until  automerge label has been added, closes #747
  (option defaults to `true`, so no breaking change)
- Make the missing automerge label message a little more friendly (see #747 (comment))
- Add tests for new option and fix other tests

I've tested this via a self hosted Kodiak instance:

First scenario:
1. Create a PR (don't add automerge label)
2. Verify the correct message (`Ignored (no automerge label: ...`) is shown on PR

Second scenario:
1. Set `missing_automerge_label_message` to `false`
2. Create a PR (don't add automerge label)
3. Verify **no** message (`Ignored (no automerge label: ...`) is shown on PR

Third scenario (addressing #747 (comment)):
1. Set `missing_automerge_label_message` to `false`
2. Create a PR (**add** automerge label)
3. Verify the correct message (`merging PR (waiting for status checks ...`) is shown on PR
4. Remove automerge label before checks have been finished
5. Verify the correct message (`Ignored (no automerge label: ...`) is shown on PR
  • Loading branch information
paescuj authored Nov 23, 2021
1 parent 93bebf9 commit c74c529
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 7 deletions.
2 changes: 2 additions & 0 deletions bot/kodiak/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,8 @@ class Merge(BaseModel):
automerge_dependencies: AutomergeDependencies = AutomergeDependencies()
# if disabled, kodiak won't require a label to queue a PR for merge
require_automerge_label: bool = True
# Show message when automerge label is missing on a PR
show_missing_automerge_label_message: bool = True
# regex to match against title and block merging. Set to empty string to
# disable check.
blacklist_title_regex: str = (
Expand Down
11 changes: 6 additions & 5 deletions bot/kodiak/evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,11 +713,12 @@ async def set_status(msg: str, markdown_content: Optional[str] = None) -> None:
and not has_automerge_label
and not should_dependency_automerge
):
await block_merge(
api,
pull_request,
f"missing automerge_label: {config.merge.automerge_label!r}",
)
await api.dequeue()
# Update status when "show_missing_automerge_label_message" is enabled or label has been removed while already in merging state
if config.merge.show_missing_automerge_label_message or merging:
await api.set_status(
f"Ignored (no automerge label: {config.merge.automerge_label!r})"
)
return

# We want users to get notified a merge conflict even if the PR matches a
Expand Down
6 changes: 6 additions & 0 deletions bot/kodiak/test/fixtures/config/config-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"usernames": []
},
"require_automerge_label": true,
"show_missing_automerge_label_message": true,
"blacklist_title_regex": ":::|||kodiak|||internal|||reserved|||:::",
"blocking_title_regex": ":::|||kodiak|||internal|||reserved|||:::",
"blacklist_labels": [],
Expand Down Expand Up @@ -254,6 +255,11 @@
"default": true,
"type": "boolean"
},
"show_missing_automerge_label_message": {
"title": "Show Missing Automerge Label Message",
"default": true,
"type": "boolean"
},
"blacklist_title_regex": {
"title": "Blacklist Title Regex",
"default": ":::|||kodiak|||internal|||reserved|||:::",
Expand Down
29 changes: 28 additions & 1 deletion bot/kodiak/test_evaluation.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,30 @@ async def test_mergeable_missing_automerge_label() -> None:
await mergeable(api=api, config=config, pull_request=pull_request)
assert api.set_status.call_count == 1
assert api.dequeue.call_count == 1
assert "cannot merge" in api.set_status.calls[0]["msg"]
assert "Ignored (no automerge label:" in api.set_status.calls[0]["msg"]

# verify we haven't tried to update/merge the PR
assert api.update_branch.called is False
assert api.merge.called is False
assert api.queue_for_merge.called is False


@pytest.mark.asyncio
async def test_mergeable_missing_automerge_label_no_message() -> None:
"""
No status message if show_missing_automerge_label_message is disabled
"""
api = create_api()
mergeable = create_mergeable()
config = create_config()
pull_request = create_pull_request()

config.merge.require_automerge_label = True
config.merge.show_missing_automerge_label_message = False
pull_request.labels = []
await mergeable(api=api, config=config, pull_request=pull_request)
assert api.set_status.call_count == 0
assert api.dequeue.call_count == 1

# verify we haven't tried to update/merge the PR
assert api.update_branch.called is False
Expand Down Expand Up @@ -544,13 +567,15 @@ async def test_mergeable_has_blacklist_labels() -> None:
config = create_config()
pull_request = create_pull_request()

config.merge.require_automerge_label = False
config.merge.blacklist_labels = ["dont merge!"]
pull_request.labels = ["bug", "dont merge!", "needs review"]

await mergeable(api=api, config=config, pull_request=pull_request)
assert api.set_status.call_count == 1
assert api.dequeue.call_count == 1
assert "cannot merge" in api.set_status.calls[0]["msg"]
assert "has blacklist_labels" in api.set_status.calls[0]["msg"]

# verify we haven't tried to update/merge the PR
assert api.update_branch.called is False
Expand All @@ -568,13 +593,15 @@ async def test_mergeable_has_blocking_labels() -> None:
config = create_config()
pull_request = create_pull_request()

config.merge.require_automerge_label = False
config.merge.blocking_labels = ["dont merge!"]
pull_request.labels = ["bug", "dont merge!", "needs review"]

await mergeable(api=api, config=config, pull_request=pull_request)
assert api.set_status.call_count == 1
assert api.dequeue.call_count == 1
assert "cannot merge" in api.set_status.calls[0]["msg"]
assert "has merge.blocking_labels" in api.set_status.calls[0]["msg"]

# verify we haven't tried to update/merge the PR
assert api.update_branch.called is False
Expand Down
2 changes: 1 addition & 1 deletion bot/kodiak/tests/evaluation/test_check_runs.py
Original file line number Diff line number Diff line change
Expand Up @@ -522,7 +522,7 @@ async def test_mergeable_update_always_require_automerge_label_missing_label() -
assert api.update_branch.call_count == 0

assert api.set_status.call_count == 1
assert "missing automerge_label:" in api.set_status.calls[0]["msg"]
assert "Ignored (no automerge label:" in api.set_status.calls[0]["msg"]
assert api.dequeue.call_count == 1

assert api.queue_for_merge.call_count == 0
Expand Down
9 changes: 9 additions & 0 deletions docs/docs/config-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ Require that the automerge label (`merge.automerge_label`) be set for Kodiak to

When disabled, Kodiak will immediately attempt to merge any PR that passes all GitHub branch protection requirements.

### `merge.show_missing_automerge_label_message`

- **type:** `boolean`
- **default:** `true`

Show a status message when automerge label is required but missing on a PR.

When disabled, no status message is shown until the automerge label has been added to the PR.

### `merge.automerge_dependencies.versions`

- **type:** `string[]`
Expand Down

0 comments on commit c74c529

Please sign in to comment.