Skip to content

Commit

Permalink
[FIX] *: unnecessary warning on r- of forward port
Browse files Browse the repository at this point in the history
Reminding users that `r-` on a forward port only unreviews *that*
forwardport is useful, as `r+;r-` is not a no-op (all preceding
siblings are still reviewed).

However it is useless if all siblings are not approved or already
merged. So avoid sending the warning in that case.

Fixes #934
  • Loading branch information
xmo-odoo committed Sep 6, 2024
1 parent 017b8d4 commit 64f9dcb
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 3 deletions.
8 changes: 8 additions & 0 deletions forwardport/tests/test_simple.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,14 @@ def test_disapproval(env, config, make_repo, users):
"sibling forward ports may have to be unapproved "
"individually."),
]
with prod:
pr2.post_comment('hansen r-', token=config['role_other']['token'])
env.run_crons(None)
assert pr2_id.state == 'validated'
assert pr2.comments[2:] == [
(users['other'], "hansen r+"),
(users['other'], "hansen r-"),
], "shouldn't have a warning on r- because it's the only approved PR of the chain"

def test_delegate_fw(env, config, make_repo, users):
"""If a user is delegated *on a forward port* they should be able to approve
Expand Down
8 changes: 5 additions & 3 deletions runbot_merge/models/pull_requests.py
Original file line number Diff line number Diff line change
Expand Up @@ -796,10 +796,12 @@ def format_help(warn_ignore: bool, address: bool = True) -> str:
pull_request=self.number,
format_args={'user': login, 'pr': self},
)
if self.source_id:
if self.source_id.forwardport_ids.filtered(
lambda p: p.reviewed_by and p.state not in ('merged', 'closed')
):
feedback("Note that only this forward-port has been"
" unapproved, sibling forward ports may "
"have to be unapproved individually.")
" unapproved, sibling forward ports may"
" have to be unapproved individually.")
self.unstage("unreviewed (r-) by %s", login)
else:
msg = "r- makes no sense in the current PR state."
Expand Down

0 comments on commit 64f9dcb

Please sign in to comment.