diff --git a/USAGE.md b/USAGE.md index 7630ab7..f408f42 100644 --- a/USAGE.md +++ b/USAGE.md @@ -3,7 +3,7 @@ This bot allows everybody to change a PRs status with a simple comment of the form `/status `. The right status of a PR depends on which group of people it is currently *actionable* to. You can set the state to - `needs_review`, if the PR author considers this PR ready and needs a peer review. Issues of this state should actionable to *anybody* who is willing to help out and spend some time on PR reviews. No special privileges or experience necessary. -- `needs_work` if the PR in its current form is not ready yet. Issues of this state are usually only actionable to the PR author, although outside help is of course also possible. +- `awaiting_changes` if the PR in its current form is not ready yet. Issues of this state are usually only actionable to the PR author, although outside help is of course also possible. - `needs_merge` can be set by reviewers who do not have merge permission but *would merge this PR if they could*. Think of this as a merge-button by proxy. PRs of this state are actionable for contributors with merge permission. These contributors may have further feedback, but the reviewer should make an honest effort to anticipate the feedback and get all issues resolved *before* setting the state to `needs_merge`. # Tips for Reviewers @@ -12,4 +12,4 @@ As explained in the previous section, any PR with the `needs_review` label shoul - If you don't understand parts of the changes: Give a review and ask the PR author for clarifications! The PR author should then usually add the clarifications as comments to the nix expression. This is very valuable feedback. In fact, sometimes it is useful to be a bit naive on purpose. All source code should be well-documented and commented. -- If you think somebody with a very specific expertise should look at the PR: Try to find out who that could be (for example by looking at similar files and who has changed them in the past) and ping them. Delegation is also an action, and often the best one. If you have done so, set the state back to `needs_work` since the PR is no longer actionable to any reviewer. If the person you pinged is unresponsive, the PR author can set the state back to `needs_review` after some reasonable amount of time (usually something in the order of 3 days to a week). +- If you think somebody with a very specific expertise should look at the PR: Try to find out who that could be (for example by looking at similar files and who has changed them in the past) and ping them. Delegation is also an action, and often the best one. If you have done so, set the state back to `awaiting_changes` since the PR is no longer actionable to any reviewer. If the person you pinged is unresponsive, the PR author can set the state back to `needs_review` after some reasonable amount of time (usually something in the order of 3 days to a week). diff --git a/marvin/__main__.py b/marvin/__main__.py index f94b16e..5f46def 100644 --- a/marvin/__main__.py +++ b/marvin/__main__.py @@ -21,7 +21,7 @@ BOT_NAME = os.environ.get("BOT_NAME", "marvin-mk2") # List of mutually exclusive status labels -ISSUE_STATUS_LABELS = {"needs_review", "needs_work", "needs_merge"} +ISSUE_STATUS_LABELS = {"needs_review", "awaiting_changes", "needs_merge"} GREETING = f""" Hi! I'm an experimental bot. My goal is to guide this PR through its stages, hopefully ending with a merge. You can read up on the usage [here](https://github.com/timokau/marvin-mk2/blob/deployed/USAGE.md). @@ -113,8 +113,8 @@ async def handle_comment( # Only handle one command for now, since a command can modify the issue and # we'd need to keep track of that. for command in commands: - if command == "status needs_work": - await set_issue_status(issue, "needs_work", gh, token) + if command == "status awaiting_changes": + await set_issue_status(issue, "awaiting_changes", gh, token) elif command == "status needs_review": await set_issue_status(issue, "needs_review", gh, token) elif command == "status needs_merge": @@ -175,7 +175,9 @@ async def pull_request_review_submitted_event( event: sansio.Event, gh: GitHubAPI, token: str, *args: Any, **kwargs: Any ) -> None: if event.data["review"]["state"] == "changes_requested": - await set_issue_status(event.data["pull_request"], "needs_work", gh, token) + await set_issue_status( + event.data["pull_request"], "awaiting_changes", gh, token + ) await handle_comment( event.data["review"], event.data["pull_request"], diff --git a/tests/test___main__.py b/tests/test___main__.py index 5959179..5c51498 100644 --- a/tests/test___main__.py +++ b/tests/test___main__.py @@ -49,7 +49,7 @@ async def test_removes_old_status_labels_on_new_status() -> None: "user": {"id": 42, "login": "somebody"}, "labels": [ {"name": "marvin"}, - {"name": "needs_work"}, + {"name": "awaiting_changes"}, {"name": "needs_merge"}, ], }, @@ -64,7 +64,7 @@ async def test_removes_old_status_labels_on_new_status() -> None: assert gh.post_data == [("issue-url/labels", {"labels": ["needs_review"]})] assert set(gh.delete_urls) == { "issue-url/labels/needs_merge", - "issue-url/labels/needs_work", + "issue-url/labels/awaiting_changes", } @@ -86,7 +86,7 @@ async def test_responds_to_pull_request_summary_commands() -> None: gh = GitHubAPIMock() await main.router.dispatch(event, gh, token="fake-token") assert gh.post_data == [ - ("pr-url/labels", {"labels": ["needs_work"]}), + ("pr-url/labels", {"labels": ["awaiting_changes"]}), ("pr-url/labels", {"labels": ["needs_review"]}), ] assert set(gh.delete_urls) == {