[CHG] forwardport: notify the outstanding forwardports rather than source

I have been convinced that this might be an improvement to the affairs
of the people: originally the message was sent to the source PR so we
wouldn't have to ping the author & reviewer and to limit the amount of
spam, *however*:

- we ended up adding pings anyway
- it also pings the followers of the source PR
- it increases the size of the original discussion (especially if was
- originally long)
- it adds steps to fixing the issue as you need to bounce from the
  source to the forward ports

Note that this might still notify a lot of people as they might be
made followers of the forward ports automatically, and it increases
the messaging load of the forwardbot significantly. But we'll see how
things go. Worst case scenario, we can revert it back.

Fixes #836
This commit is contained in:
Xavier Morel 2024-06-03 15:29:47 +02:00
parent 9c51f87aed
commit 98aaa9107f
3 changed files with 67 additions and 27 deletions

View File

@ -553,17 +553,12 @@ stderr:
if source.merge_date > (cutoff_dt - backoff):
continue
source.reminder_backoff_factor += 1
for pr in prs:
self.env.ref('runbot_merge.forwardport.reminder')._send(
repository=source.repository,
pull_request=source.number,
repository=pr.repository,
pull_request=pr.number,
token_field='fp_github_token',
format_args={
'pr': source,
'outstanding': ''.join(
f'\n- {pr.display_name}'
for pr in sorted(prs, key=lambda p: p.number)
),
}
format_args={'pr': pr, 'source': source},
)
class Stagings(models.Model):

View File

@ -6,7 +6,7 @@ from datetime import datetime, timedelta
import pytest
from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr
from utils import seen, Commit, make_basic, REF_PATTERN, MESSAGE_TEMPLATE, validate_all, part_of, to_pr, re_matches
FMT = '%Y-%m-%d %H:%M:%S'
FAKE_PREV_WEEK = (datetime.now() + timedelta(days=1)).strftime(FMT)
@ -132,10 +132,20 @@ def test_straightforward_flow(env, config, make_repo, users):
(users['reviewer'], 'hansen r+ rebase-ff'),
seen(env, pr, users),
(users['user'], 'Merge method set to rebase and fast-forward.'),
(users['user'], '@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n- %s' % (
users['other'], users['reviewer'],
'\n- '.join((pr1 | pr2).mapped('display_name'))
)),
]
pr1_remote = prod.get_pr(pr1.number)
assert pr1_remote.comments == [
seen(env, pr1_remote, users),
(users['user'], """\
This PR targets b and is part of the forward-port chain. Further PRs will be created up to c.
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
"""),
(users['user'], "@%s @%s this forward port of %s is awaiting action (not merged or closed)." % (
users['other'],
users['reviewer'],
pr0.display_name,
))
]
assert pr0_ == pr0
@ -168,6 +178,11 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
users['other'], users['reviewer'],
pr1.display_name,
)),
(users['user'], "@%s @%s this forward port of %s is awaiting action (not merged or closed)." % (
users['other'],
users['reviewer'],
pr0.display_name,
))
]
with prod:
prod.post_status(pr2.head, 'success', 'ci/runbot')
@ -231,7 +246,7 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
assert other_user_repo.get_ref(pr.ref) == p_1
# should have deleted all PR branches
pr1_ref = prod.get_pr(pr1.number).ref
pr1_ref = pr1_remote.ref
with pytest.raises(AssertionError, match='Not Found'):
other.get_ref(pr1_ref)
@ -326,27 +341,57 @@ def test_empty(env, config, make_repo, users):
awaiting = (
users['other'],
'@%s @%s this pull request has forward-port PRs awaiting action (not merged or closed):\n- %s' % (
'@%s @%s this forward port of %s is awaiting action (not merged or closed).' % (
users['user'], users['reviewer'],
fail_id.display_name
pr1_id.display_name
)
)
conflict = (users['user'], re_matches(
fr"""@{users['user']} @{users['reviewer']} cherrypicking of pull request {pr1_id.display_name} failed\.
stdout:
```
.*
```
stderr:
```
.*
```
Either perform the forward-port manually \(and push to this branch, proceeding as usual\) or close this PR \(maybe\?\)\.
In the former case, you may want to edit this PR message as well\.
:warning: after resolving this conflict, you will need to merge it via @{project.github_prefix}\.
More info at https://github\.com/odoo/odoo/wiki/Mergebot#forward-port
""", re.DOTALL))
assert pr1.comments == [
(users['reviewer'], 'hansen r+'),
seen(env, pr1, users),
]
fail_pr = prod.get_pr(fail_id.number)
assert fail_pr.comments == [
seen(env, fail_pr, users),
conflict,
awaiting,
awaiting,
], "each cron run should trigger a new message on the ancestor"
], "each cron run should trigger a new message"
# check that this stops if we close the PR
with prod:
prod.get_pr(fail_id.number).close()
fail_pr.close()
env.run_crons('forwardport.reminder', 'runbot_merge.feedback_cron', context={'forwardport_updated_before': FAKE_PREV_WEEK})
assert pr1.comments == [
(users['reviewer'], 'hansen r+'),
seen(env, pr1, users),
awaiting,
awaiting,
]
assert fail_pr.comments == [
seen(env, fail_pr, users),
conflict,
awaiting,
awaiting,
], "each cron run should trigger a new message"
def test_partially_empty(env, config, make_repo):
""" Check what happens when only some commits of the PR are now empty

View File

@ -131,10 +131,10 @@ runbot_merge.forwardport.failure.conflict,"{pr.ping}the next pull request ({new.
pr: the pr which was just forward ported
new: the new forward-port
footer: some footer text"
runbot_merge.forwardport.reminder,{pr.ping}this pull request has forward-port PRs awaiting action (not merged or closed):{outstanding},"Comment when a forward port has outstanding (not merged or closed) descendants
runbot_merge.forwardport.reminder,{pr.ping}this forward port of {source.display_name} is awaiting action (not merged or closed).,"Comment when a forward port has outstanding (not merged or closed) descendants
pr: the original (source) PR
outstanding: markdown-formatted list of outstanding PRs"
pr: the forward-port
source: the source PR"
runbot_merge.forwardport.failure,"{pr.ping}cherrypicking of pull request {pr.source_id.display_name} failed.
{commits}{stdout}{stderr}
Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

1 id template help
131
132
133
134
135
136
137
138
139
140