mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 23:45:44 +07:00
[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:
parent
9c51f87aed
commit
98aaa9107f
@ -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):
|
||||
|
@ -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
|
||||
|
@ -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?).
|
||||
|
|
Loading…
Reference in New Issue
Block a user