Repository navigation
Remove any "awaiting.." labels when a PR is merged. - #58
Conversation
brettcannon
left a comment
brettcannon
left a comment
There was a problem hiding this comment.
The approach LGTM! Some minor PEP 8 things, a failing test, and there should be an explicit test in test_stage for the change.
|
|
||
| @router.register("pull_request", action="closed") | ||
| async def closed_pr(event, gh, *args, **kwargs): | ||
| """Remove all `awaiting ... ` label when PR was merged""" |
| issue = await util.issue_for_PR(gh, event.data["pull_request"]) | ||
| for label_data in issue["labels"]: | ||
| if LABEL_PREFIX in label_data['name']: | ||
| await gh.delete(label_data["url"]) No newline at end of file |
| "issue_url": "https://api.github.com/repos/python/cpython/issues/10", | ||
| "number": 10, | ||
| "state": "closed", | ||
| "merged": True |
| # Sending a payload that shouldn't trigger any networking, but no errors | ||
| # either. | ||
| data = {"action": "closed"} | ||
| data = { |
There was a problem hiding this comment.
This test is now failing, probably because closing a PR now triggers networking calls (see the comment in line 24). It probably needs to perform an action that isn't registered with the router to perform the same intent in testing.
Codecov Report
@@ Coverage Diff @@
## master #58 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 14 14
Lines 850 872 +22
Branches 49 50 +1
=====================================
+ Hits 850 872 +22
Continue to review full report at Codecov.
|
|
Just in case you didn't notice, codecov says you're no longer exercising the new code. |
|
Thanks! The coverage looks good now 😃 |
|
I did one minor refactoring as I realized that there were two places in the code now trying to remove labels. Once Travis verifies I didn't botch anything you can feel free to merge! |
|
OK, everything should be good but feel free to double-check me. And I think the initial logic was wrong because I believe you were going to delete the label from the entire repository instead of remove it from the issue initially. 😉 |
|
😱 |
|
|
||
|
|
||
| async def _remove_stage_labels(gh, issue): | ||
| """Remove all "awaiting" labels.""" |
There was a problem hiding this comment.
Should use 4 space indentation.
|
Yep, there's a webhook so that Heroku will pick up any changes to the master branch and automatically deploy it. |
|
Thanks! Then I'll merge this now 😁 |
|
It works! python/cpython#3886 (comment) |
Closes #53