[FIX] *: unnecessary warning on r- of forward port

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
This commit is contained in:
Xavier Morel 2024-09-06 13:04:13 +02:00
parent 017b8d4bb0
commit 64f9dcbc22
2 changed files with 13 additions and 3 deletions

View File

@ -629,6 +629,14 @@ def test_disapproval(env, config, make_repo, users):
"sibling forward ports may have to be unapproved " "sibling forward ports may have to be unapproved "
"individually."), "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): def test_delegate_fw(env, config, make_repo, users):
"""If a user is delegated *on a forward port* they should be able to approve """If a user is delegated *on a forward port* they should be able to approve

View File

@ -796,10 +796,12 @@ For your own safety I've ignored *everything in your entire comment*.
pull_request=self.number, pull_request=self.number,
format_args={'user': login, 'pr': self}, 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" feedback("Note that only this forward-port has been"
" unapproved, sibling forward ports may " " unapproved, sibling forward ports may"
"have to be unapproved individually.") " have to be unapproved individually.")
self.unstage("unreviewed (r-) by %s", login) self.unstage("unreviewed (r-) by %s", login)
else: else:
msg = "r- makes no sense in the current PR state." msg = "r- makes no sense in the current PR state."