Skip to content

Commit

Permalink
Don't merge PR unless it has "awaiting merge" label (#112)
Browse files Browse the repository at this point in the history
Change the behavior for when deciding when miss-islington
should merge the PR.

Previously it checks if the PR has one core dev who approved the PR.
But that is unreliable because the review may be dismissed later,
or another core dev might decide not to backport the PR anymore.

The "awaiting merge" label is a better indicator that the PR
is ready for merge.

Adjust tests.

Fixes python/miss-islington#62
  • Loading branch information
Mariatta authored Jun 29, 2018
1 parent 863dde5 commit 8fe9d1f
Show file tree
Hide file tree
Showing 5 changed files with 230 additions and 117 deletions.
22 changes: 9 additions & 13 deletions miss_islington/status_change.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,11 @@ async def check_status(event, gh, *args, **kwargs):
await check_ci_status_and_approval(gh, sha, leave_comment=True)


@router.register("pull_request_review", action="submitted")
@router.register("pull_request", action="labeled")
async def pr_reviewed(event, gh, *args, **kwargs):
if event.data["pull_request"]["user"]["login"] == "miss-islington":
reviewer = event.data["review"]["user"]["login"]
approved = event.data["review"]["state"] == "approved"
if approved and await util.is_core_dev(gh, reviewer):
sha = event.data["review"]["commit_id"]
if util.pr_is_awaiting_merge(event.data["pull_request"]["labels"]):
sha = event.data["pull_request"]["head"]["sha"]
await check_ci_status_and_approval(gh, sha)


Expand Down Expand Up @@ -75,14 +73,12 @@ async def check_ci_status_and_approval(gh, sha, leave_comment=False):
)

if result["state"] == "success":
async for review in gh.getiter(
f"/repos/python/cpython/pulls/{pr_number}/reviews"
):
reviewer = review["user"]["login"]
approved = review["state"].lower() == "approved"
if approved and await util.is_core_dev(gh, reviewer):
await merge_pr(gh, pr_number, sha)
break
pr = await gh.getitem(
f"/repos/python/cpython/pulls/{pr_number}"
)
if util.pr_is_awaiting_merge(pr["labels"]):
await merge_pr(gh, pr_number, sha)
break


async def merge_pr(gh, pr_number, sha):
Expand Down
7 changes: 7 additions & 0 deletions miss_islington/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,10 @@ async def is_core_dev(gh, username):
raise
else:
return True


def pr_is_awaiting_merge(pr_labels):
for label in pr_labels:
if label["name"] == "awaiting merge":
return True
return False
4 changes: 0 additions & 4 deletions tests/test_delete_branch.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@ class FakeGH:
def __init__(self):
self.post_data = None

async def post(self, url, *, data):
self.post_url = url
self.post_data = data

async def delete(self, url):
self.delete_url = url

Expand Down
Loading

0 comments on commit 8fe9d1f

Please sign in to comment.