mirror of
https://github.com/odoo/runbot.git
synced 2025-03-15 23:45:44 +07:00
[REF] *: move most feedback messages to pseudo-templates
Current system makes it hard to iterate feedback messages and make them clearer, this should improve things a touch. Use a bespoke model to avoid concerns with qweb rendering complexity (we just want GFM output and should not need logic). Also update fwbot test setup to always configure an fwbot name, in order to avoid ping messages closing the PRs they're talking about, that took a while to debug, and given the old message I assume I'd already hit it and just been too lazy to fix. This requires updating a bunch of tests as fwbot ping are sent *to* `fp_github_name`, but sent *from* the reference user (because that's the key we set). Note: noupdate on CSV files doesn't seem to work anymore, which isn't great. But instead set tracking on the template's templates, it's not quite as good but should be sufficient. Fixes #769
This commit is contained in:
Xavier Morel
parent
e14616b2fb
commit
270dfdd495
@ -98,14 +98,6 @@ class ForwardPortTasks(models.Model, Queue):
|
||||
batch.active = False
|
||||
|
||||
|
||||
CONFLICT_TEMPLATE = "{ping}WARNING: the latest change ({previous.head}) triggered " \
|
||||
"a conflict when updating the next forward-port " \
|
||||
"({next.display_name}), and has been ignored.\n\n" \
|
||||
"You will need to update this pull request differently, " \
|
||||
"or fix the issue by hand on {next.display_name}."
|
||||
CHILD_CONFLICT = "{ping}WARNING: the update of {previous.display_name} to " \
|
||||
"{previous.head} has caused a conflict in this pull request, " \
|
||||
"data may have been lost."
|
||||
class UpdateQueue(models.Model, Queue):
|
||||
_name = 'forwardport.updates'
|
||||
_description = 'if a forward-port PR gets updated & has followups (cherrypick succeeded) the followups need to be updated as well'
|
||||
@ -116,7 +108,6 @@ class UpdateQueue(models.Model, Queue):
|
||||
new_root = fields.Many2one('runbot_merge.pull_requests')
|
||||
|
||||
def _process_item(self):
|
||||
Feedback = self.env['runbot_merge.pull_requests.feedback']
|
||||
previous = self.new_root
|
||||
with ExitStack() as s:
|
||||
for child in self.new_root._iter_descendants():
|
||||
@ -134,41 +125,35 @@ class UpdateQueue(models.Model, Queue):
|
||||
self.new_root.display_name
|
||||
)
|
||||
if child.state in ('closed', 'merged'):
|
||||
Feedback.create({
|
||||
'repository': child.repository.id,
|
||||
'pull_request': child.number,
|
||||
'message': "%sancestor PR %s has been updated but this PR"
|
||||
" is %s and can't be updated to match."
|
||||
"\n\n"
|
||||
"You may want or need to manually update any"
|
||||
" followup PR." % (
|
||||
child.ping(),
|
||||
self.new_root.display_name,
|
||||
child.state,
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.updates.closed')._send(
|
||||
repository=child.repository,
|
||||
pull_request=child.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': child, 'parent': self.new_root},
|
||||
)
|
||||
return
|
||||
|
||||
conflicts, working_copy = previous._create_fp_branch(
|
||||
child.target, child.refname, s)
|
||||
if conflicts:
|
||||
_, out, err, _ = conflicts
|
||||
Feedback.create({
|
||||
'repository': previous.repository.id,
|
||||
'pull_request': previous.number,
|
||||
'message': CONFLICT_TEMPLATE.format(
|
||||
ping=previous.ping(),
|
||||
previous=previous,
|
||||
next=child
|
||||
)
|
||||
})
|
||||
Feedback.create({
|
||||
'repository': child.repository.id,
|
||||
'pull_request': child.number,
|
||||
'message': CHILD_CONFLICT.format(ping=child.ping(), previous=previous, next=child)\
|
||||
+ (f'\n\nstdout:\n```\n{out.strip()}\n```' if out.strip() else '')
|
||||
+ (f'\n\nstderr:\n```\n{err.strip()}\n```' if err.strip() else '')
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.updates.conflict.parent')._send(
|
||||
repository=previous.repository,
|
||||
pull_request=previous.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': previous, 'next': child},
|
||||
)
|
||||
self.env.ref('runbot_merge.forwardport.updates.conflict.child')._send(
|
||||
repository=child.repository,
|
||||
pull_request=child.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={
|
||||
'previous': previous,
|
||||
'pr': child,
|
||||
'stdout': (f'\n\nstdout:\n```\n{out.strip()}\n```' if out.strip() else ''),
|
||||
'stderr': (f'\n\nstderr:\n```\n{err.strip()}\n```' if err.strip() else ''),
|
||||
},
|
||||
)
|
||||
|
||||
new_head = working_copy.stdout().rev_parse(child.refname).stdout.decode().strip()
|
||||
commits_count = int(working_copy.stdout().rev_list(
|
||||
|
||||
@ -67,8 +67,9 @@ class Project(models.Model):
|
||||
def _compute_git_identity(self):
|
||||
s = requests.Session()
|
||||
for project in self:
|
||||
if not project.fp_github_token:
|
||||
if not project.fp_github_token or (self.fp_github_name and self.fp_github_email):
|
||||
continue
|
||||
|
||||
r0 = s.get('https://api.github.com/user', headers={
|
||||
'Authorization': 'token %s' % project.fp_github_token
|
||||
})
|
||||
@ -246,6 +247,25 @@ class PullRequests(models.Model):
|
||||
for pr in self:
|
||||
pr.refname = pr.label.split(':', 1)[-1]
|
||||
|
||||
ping = fields.Char(recursive=True)
|
||||
|
||||
@api.depends('source_id.author.github_login', 'source_id.reviewed_by.github_login')
|
||||
def _compute_ping(self):
|
||||
"""For forward-port PRs (PRs with a source) the author is the PR bot, so
|
||||
we want to ignore that and use the author & reviewer of the original PR
|
||||
"""
|
||||
source = self.source_id
|
||||
if not source:
|
||||
return super()._compute_ping()
|
||||
|
||||
for pr in self:
|
||||
s = ' '.join(
|
||||
f'@{p.github_login}'
|
||||
for p in source.author | source.reviewed_by | self.reviewed_by
|
||||
)
|
||||
pr.ping = s and (s + ' ')
|
||||
|
||||
|
||||
@api.model_create_single
|
||||
def create(self, vals):
|
||||
# PR opened event always creates a new PR, override so we can precreate PRs
|
||||
@ -297,32 +317,24 @@ class PullRequests(models.Model):
|
||||
if self.env.context.get('forwardport_detach_warn', True):
|
||||
for p in with_parents:
|
||||
if not p.parent_id:
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': p.repository.id,
|
||||
'pull_request': p.number,
|
||||
'message': "%sthis PR was modified / updated and has become a normal PR. "
|
||||
"It should be merged the normal way (via @%s)" % (
|
||||
p.source_id.ping(),
|
||||
p.repository.project_id.github_prefix,
|
||||
),
|
||||
'token_field': 'fp_github_token',
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.update.detached')._send(
|
||||
repository=p.repository,
|
||||
pull_request=p.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': p},
|
||||
)
|
||||
for p in closed_fp.filtered(lambda p: p.state != 'closed'):
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': p.repository.id,
|
||||
'pull_request': p.number,
|
||||
'message': "%sthis PR was closed then reopened. "
|
||||
"It should be merged the normal way (via @%s)" % (
|
||||
p.source_id.ping(),
|
||||
p.repository.project_id.github_prefix,
|
||||
),
|
||||
'token_field': 'fp_github_token',
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.reopen.detached')._send(
|
||||
repository=p.repository,
|
||||
pull_request=p.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': p},
|
||||
)
|
||||
if vals.get('state') == 'merged':
|
||||
for p in self:
|
||||
self.env['forwardport.branch_remover'].create({
|
||||
'pr_id': p.id,
|
||||
})
|
||||
self.env['forwardport.branch_remover'].create([
|
||||
{'pr_id': p.id}
|
||||
for p in self
|
||||
])
|
||||
# if we change the policy to skip CI, schedule followups on existing FPs
|
||||
if vals.get('fw_policy') == 'skipci' and self.state == 'merged':
|
||||
self.env['runbot_merge.pull_requests'].search([
|
||||
@ -455,15 +467,12 @@ class PullRequests(models.Model):
|
||||
if not (self.state == 'opened' and self.parent_id):
|
||||
return
|
||||
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'token_field': 'fp_github_token',
|
||||
'message': '%s%s failed on this forward-port PR' % (
|
||||
self.source_id.ping(),
|
||||
ci,
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.ci.failed')._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': self, 'ci': ci},
|
||||
)
|
||||
|
||||
def _validate(self, statuses):
|
||||
failed = super()._validate(statuses)
|
||||
@ -636,16 +645,12 @@ class PullRequests(models.Model):
|
||||
linked, other = different_pr, different_target
|
||||
if t != target:
|
||||
linked, other = ref, target
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'token_field': 'fp_github_token',
|
||||
'message': "%sthis pull request can not be forward ported: "
|
||||
"next branch is %r but linked pull request %s "
|
||||
"has a next branch %r." % (
|
||||
pr.ping(), t.name, linked.display_name, other.name
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.failure.discrepancy')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'pr': pr, 'linked': linked, 'next': t.name, 'other': other.name},
|
||||
)
|
||||
_logger.warning(
|
||||
"Cancelling forward-port of %s: found different next branches (%s)",
|
||||
self, all_targets
|
||||
@ -743,23 +748,18 @@ class PullRequests(models.Model):
|
||||
'delegates': [(6, False, (source.delegates | pr.delegates).ids)]
|
||||
})
|
||||
if has_conflicts and pr.parent_id and pr.state not in ('merged', 'closed'):
|
||||
message = source.ping() + """\
|
||||
the next pull request (%s) is in conflict. You can merge the chain up to here by saying
|
||||
> @%s r+
|
||||
%s""" % (new_pr.display_name, pr.repository.project_id.fp_github_name, footer)
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': message,
|
||||
'token_field': 'fp_github_token',
|
||||
})
|
||||
self.env.ref('runbot_merge.forwardport.failure.conflict')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
token_field='fp_github_token',
|
||||
format_args={'source': source, 'pr': pr, 'new': new_pr, 'footer': footer},
|
||||
)
|
||||
# not great but we probably want to avoid the risk of the webhook
|
||||
# creating the PR from under us. There's still a "hole" between
|
||||
# the POST being executed on gh and the commit but...
|
||||
self.env.cr.commit()
|
||||
|
||||
for pr, new_pr in zip(self, new_batch):
|
||||
source = pr.source_id or pr
|
||||
(h, out, err, hh) = conflicts.get(pr) or (None, None, None, None)
|
||||
|
||||
if h:
|
||||
@ -775,47 +775,47 @@ the next pull request (%s) is in conflict. You can merge the chain up to here by
|
||||
'* %s%s\n' % (sha, ' <- on this commit' if sha == h else '')
|
||||
for sha in hh
|
||||
)
|
||||
message = f"""{source.ping()}cherrypicking of pull request {source.display_name} failed.
|
||||
{lines}{sout}{serr}
|
||||
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.
|
||||
"""
|
||||
template = 'runbot_merge.forwardport.failure'
|
||||
format_args = {
|
||||
'pr': new_pr,
|
||||
'commits': lines,
|
||||
'stdout': sout,
|
||||
'stderr': serr,
|
||||
'footer': footer,
|
||||
}
|
||||
elif has_conflicts:
|
||||
message = """%s\
|
||||
while this was properly forward-ported, at least one co-dependent PR (%s) did \
|
||||
not succeed. You will need to fix it before this can be merged.
|
||||
|
||||
Both this PR and the others will need to be approved via `@%s r+` as they are \
|
||||
all considered "in conflict".
|
||||
%s""" % (
|
||||
source.ping(),
|
||||
', '.join(p.display_name for p in (new_batch - new_pr)),
|
||||
proj.github_prefix,
|
||||
footer
|
||||
)
|
||||
template = 'runbot_merge.forwardport.linked'
|
||||
format_args = {
|
||||
'pr': new_pr,
|
||||
'siblings': ', '.join(p.display_name for p in (new_batch - new_pr)),
|
||||
'footer': footer,
|
||||
}
|
||||
elif base._find_next_target(new_pr) is None:
|
||||
ancestors = "".join(
|
||||
"* %s\n" % p.display_name
|
||||
for p in pr._iter_ancestors()
|
||||
if p.parent_id
|
||||
)
|
||||
message = source.ping() + """\
|
||||
this PR targets %s and is the last of the forward-port chain%s
|
||||
%s
|
||||
To merge the full chain, say
|
||||
> @%s r+
|
||||
%s""" % (target.name, ' containing:' if ancestors else '.', ancestors, pr.repository.project_id.fp_github_name, footer)
|
||||
template = 'runbot_merge.forwardport.final'
|
||||
format_args = {
|
||||
'pr': new_pr,
|
||||
'containing': ' containing:' if ancestors else '.',
|
||||
'ancestors': ancestors,
|
||||
'footer': footer,
|
||||
}
|
||||
else:
|
||||
message = """\
|
||||
This PR targets %s and is part of the forward-port chain. Further PRs will be created up to %s.
|
||||
%s""" % (target.name, base.limit_id.name, footer)
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': new_pr.repository.id,
|
||||
'pull_request': new_pr.number,
|
||||
'message': message,
|
||||
'token_field': 'fp_github_token',
|
||||
})
|
||||
template = 'runbot_merge.forwardport.intermediate'
|
||||
format_args = {
|
||||
'pr': new_pr,
|
||||
'footer': footer,
|
||||
}
|
||||
self.env.ref(template)._send(
|
||||
repository=new_pr.repository,
|
||||
pull_request=new_pr.number,
|
||||
token_field='fp_github_token',
|
||||
format_args=format_args,
|
||||
)
|
||||
|
||||
labels = ['forwardport']
|
||||
if has_conflicts:
|
||||
labels.append('conflict')
|
||||
@ -1150,31 +1150,18 @@ stderr:
|
||||
if source.merge_date > (cutoff_dt - backoff):
|
||||
continue
|
||||
source.reminder_backoff_factor += 1
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': source.repository.id,
|
||||
'pull_request': source.number,
|
||||
'message': "%sthis pull request has forward-port PRs awaiting action (not merged or closed):\n%s" % (
|
||||
source.ping(),
|
||||
'\n- '.join(pr.display_name for pr in sorted(prs, key=lambda p: p.number))
|
||||
),
|
||||
'token_field': 'fp_github_token',
|
||||
})
|
||||
|
||||
def ping(self, author=True, reviewer=True):
|
||||
source = self.source_id
|
||||
if not source:
|
||||
return super().ping(author=author, reviewer=reviewer)
|
||||
|
||||
# use a dict literal to maintain ordering (3.6+)
|
||||
pingline = ' '.join(
|
||||
f'@{p.github_login}'
|
||||
for p in filter(None, {
|
||||
author and source.author: None,
|
||||
reviewer and source.reviewed_by: None,
|
||||
reviewer and self.reviewed_by: None,
|
||||
})
|
||||
)
|
||||
return pingline and (pingline + ' ')
|
||||
self.env.ref('runbot_merge.forwardport.reminder')._send(
|
||||
repository=source.repository,
|
||||
pull_request=source.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)
|
||||
),
|
||||
}
|
||||
)
|
||||
|
||||
class Stagings(models.Model):
|
||||
_inherit = 'runbot_merge.stagings'
|
||||
|
||||
@ -316,11 +316,11 @@ def test_multiple_commits_different_authorship(env, config, make_repo, users, ro
|
||||
c = prod.commit(pr2_id.head)
|
||||
assert len(c.parents) == 1
|
||||
get = itemgetter('name', 'email')
|
||||
rm = rolemap['user']
|
||||
assert get(c.author) == (rm['login'], ''), \
|
||||
bot = pr_id.repository.project_id.fp_github_name
|
||||
assert get(c.author) == (bot, ''), \
|
||||
"In a multi-author PR, the squashed conflict commit should have the " \
|
||||
"author set to the bot but an empty email"
|
||||
assert get(c.committer) == (rm['login'], '')
|
||||
assert get(c.committer) == (bot, '')
|
||||
|
||||
assert re.match(r'''<<<\x3c<<< HEAD
|
||||
b
|
||||
|
||||
@ -174,7 +174,7 @@ def test_limit_after_merge(env, config, make_repo, users):
|
||||
(users['reviewer'], "hansen r+"),
|
||||
seen(env, pr1, users),
|
||||
(users['reviewer'], bot_name + ' up to b'),
|
||||
(bot_name, "@%s forward-port limit can only be set before the PR is merged." % users['reviewer']),
|
||||
(users['user'], "@%s forward-port limit can only be set before the PR is merged." % users['reviewer']),
|
||||
]
|
||||
assert pr2.comments == [
|
||||
seen(env, pr2, users),
|
||||
@ -184,7 +184,7 @@ This PR targets b and is part of the forward-port chain. Further PRs will be cre
|
||||
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
||||
"""),
|
||||
(users['reviewer'], bot_name + ' up to b'),
|
||||
(bot_name, "@%s forward-port limit can only be set on an origin PR"
|
||||
(users['user'], "@%s forward-port limit can only be set on an origin PR"
|
||||
" (%s here) before it's merged and forward-ported." % (
|
||||
users['reviewer'],
|
||||
p1.display_name,
|
||||
@ -208,13 +208,14 @@ More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
||||
env.run_crons()
|
||||
|
||||
assert pr2.comments[4:] == [
|
||||
(bot_name, "@%s @%s this PR was modified / updated and has become a normal PR. "
|
||||
(users['user'], "@%s @%s this PR was modified / updated and has become a normal PR. "
|
||||
"It should be merged the normal way (via @%s)" % (
|
||||
users['user'], users['reviewer'],
|
||||
p2.repository.project_id.github_prefix
|
||||
)),
|
||||
(users['reviewer'], bot_name + ' up to b'),
|
||||
(bot_name, f"@{users['reviewer']} forward-port limit can only be set on an origin PR "
|
||||
f"({p1.display_name} here) before it's merged and forward-ported."
|
||||
),
|
||||
(users['user'], f"@{users['reviewer']} forward-port limit can only be "
|
||||
f"set on an origin PR ({p1.display_name} here) before "
|
||||
f"it's merged and forward-ported."
|
||||
),
|
||||
]
|
||||
|
||||
@ -109,7 +109,7 @@ def test_straightforward_flow(env, config, make_repo, users):
|
||||
assert c.author['name'] == other_user['user'], "author should still be original's probably"
|
||||
assert c.committer['name'] == other_user['user'], "committer should also still be the original's, really"
|
||||
|
||||
assert pr1.ping() == "@%s @%s " % (
|
||||
assert pr1.ping == "@%s @%s " % (
|
||||
config['role_other']['user'],
|
||||
config['role_reviewer']['user'],
|
||||
), "ping of forward-port PR should include author and reviewer of source"
|
||||
@ -132,7 +132,7 @@ 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['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'))
|
||||
)),
|
||||
@ -160,7 +160,7 @@ def test_straightforward_flow(env, config, make_repo, users):
|
||||
@%s @%s this PR targets c and is the last of the forward-port chain containing:
|
||||
* %s
|
||||
|
||||
To merge the full chain, say
|
||||
To merge the full chain, use
|
||||
> @%s r+
|
||||
|
||||
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
||||
@ -315,7 +315,11 @@ def test_empty(env, config, make_repo, users):
|
||||
assert env['runbot_merge.pull_requests'].search([], order='number') == prs
|
||||
# change FP token to see if the feedback comes from the proper user
|
||||
project = env['runbot_merge.project'].search([])
|
||||
project.fp_github_token = config['role_other']['token']
|
||||
project.write({
|
||||
'fp_github_name': False,
|
||||
'fp_github_email': False,
|
||||
'fp_github_token': config['role_other']['token'],
|
||||
})
|
||||
assert project.fp_github_name == users['other']
|
||||
|
||||
# check reminder
|
||||
@ -324,7 +328,7 @@ 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 pull request has forward-port PRs awaiting action (not merged or closed):\n- %s' % (
|
||||
users['user'], users['reviewer'],
|
||||
fail_id.display_name
|
||||
)
|
||||
@ -582,11 +586,11 @@ def test_delegate_fw(env, config, make_repo, users):
|
||||
seen(env, pr2, users),
|
||||
(users['user'], '''@{self_reviewer} @{reviewer} this PR targets c and is the last of the forward-port chain.
|
||||
|
||||
To merge the full chain, say
|
||||
> @{user} r+
|
||||
To merge the full chain, use
|
||||
> @{bot} r+
|
||||
|
||||
More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port
|
||||
'''.format_map(users)),
|
||||
'''.format(bot=pr1_id.repository.project_id.fp_github_name, **users)),
|
||||
(users['other'], 'hansen r+')
|
||||
]
|
||||
|
||||
|
||||
@ -3,9 +3,6 @@ Test cases for updating PRs during after the forward-porting process after the
|
||||
initial merge has succeeded (and forward-porting has started)
|
||||
"""
|
||||
import re
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
from utils import seen, re_matches, Commit, make_basic, to_pr
|
||||
|
||||
@ -248,6 +245,8 @@ def test_duplicate_fw(env, make_repo, setreviewers, config, users):
|
||||
'github_token': config['github']['token'],
|
||||
'github_prefix': 'hansen',
|
||||
'fp_github_token': config['github']['token'],
|
||||
'fp_github_name': 'herbert',
|
||||
'fp_github_email': 'hb@example.com',
|
||||
'branch_ids': [
|
||||
(0, 0, {'name': 'master', 'sequence': 0}),
|
||||
(0, 0, {'name': 'v3', 'sequence': 1}),
|
||||
|
||||
@ -25,6 +25,8 @@ def make_basic(env, config, make_repo, *, fp_token, fp_remote):
|
||||
'github_token': config['github']['token'],
|
||||
'github_prefix': 'hansen',
|
||||
'fp_github_token': fp_token and config['github']['token'],
|
||||
'fp_github_name': 'herbert',
|
||||
'fp_github_email': 'hb@example.com',
|
||||
'branch_ids': [
|
||||
(0, 0, {'name': 'a', 'sequence': 2}),
|
||||
(0, 0, {'name': 'b', 'sequence': 1}),
|
||||
@ -262,6 +264,8 @@ class TestNotAllBranches:
|
||||
'github_token': config['github']['token'],
|
||||
'github_prefix': 'hansen',
|
||||
'fp_github_token': config['github']['token'],
|
||||
'fp_github_name': 'herbert',
|
||||
'fp_github_email': 'hb@example.com',
|
||||
'branch_ids': [
|
||||
(0, 0, {'name': 'a', 'sequence': 2}),
|
||||
(0, 0, {'name': 'b', 'sequence': 1}),
|
||||
@ -401,7 +405,7 @@ class TestNotAllBranches:
|
||||
assert pr_a.comments == [
|
||||
(users['reviewer'], 'hansen r+'),
|
||||
seen(env, pr_a, users),
|
||||
(users['user'], "@%s @%s this pull request can not be forward ported:"
|
||||
(users['user'], "@%s @%s this pull request can not be forward-ported:"
|
||||
" next branch is 'b' but linked pull request %s "
|
||||
"has a next branch 'c'." % (
|
||||
users['user'], users['reviewer'], pr_b_id.display_name,
|
||||
@ -410,7 +414,7 @@ class TestNotAllBranches:
|
||||
assert pr_b.comments == [
|
||||
(users['reviewer'], 'hansen r+'),
|
||||
seen(env, pr_b, users),
|
||||
(users['user'], "@%s @%s this pull request can not be forward ported:"
|
||||
(users['user'], "@%s @%s this pull request can not be forward-ported:"
|
||||
" next branch is 'c' but linked pull request %s "
|
||||
"has a next branch 'b'." % (
|
||||
users['user'], users['reviewer'], pr_a_id.display_name,
|
||||
@ -472,6 +476,7 @@ def test_new_intermediate_branch(env, config, make_repo):
|
||||
with prod:
|
||||
validate(prod, pr0_fp_id.head)
|
||||
env.run_crons()
|
||||
assert pr0_fp_id.state == 'validated'
|
||||
original0 = PRs.search([('parent_id', '=', pr0_fp_id.id)])
|
||||
assert original0, "Could not find FP of PR0 to C"
|
||||
assert original0.target.name == 'c'
|
||||
@ -519,6 +524,7 @@ def test_new_intermediate_branch(env, config, make_repo):
|
||||
})
|
||||
env.run_crons()
|
||||
|
||||
assert pr0_fp_id.state == 'validated'
|
||||
# created an intermediate PR for 0 and x
|
||||
desc0 = PRs.search([('source_id', '=', pr0_id.id)])
|
||||
new0 = desc0 - pr0_fp_id - original0
|
||||
|
||||
@ -73,6 +73,8 @@ def make_basic(env, config, make_repo, *, reponame='proj', project_name='myproje
|
||||
'github_token': config['github']['token'],
|
||||
'github_prefix': 'hansen',
|
||||
'fp_github_token': config['github']['token'],
|
||||
'fp_github_name': 'herbert',
|
||||
'fp_github_email': 'hb@example.com',
|
||||
'branch_ids': [
|
||||
(0, 0, {'name': 'a', 'sequence': 100}),
|
||||
(0, 0, {'name': 'b', 'sequence': 80}),
|
||||
|
||||
@ -7,6 +7,7 @@
|
||||
'security/ir.model.access.csv',
|
||||
|
||||
'data/merge_cron.xml',
|
||||
'data/runbot_merge.pull_requests.feedback.template.csv',
|
||||
'views/res_partner.xml',
|
||||
'views/runbot_merge_project.xml',
|
||||
'views/mergebot.xml',
|
||||
|
||||
@ -143,11 +143,19 @@ def handle_pr(env, event):
|
||||
|
||||
message = None
|
||||
if not branch:
|
||||
message = f"This PR targets the un-managed branch {r}:{b}, it needs to be retargeted before it can be merged."
|
||||
message = env.ref('runbot_merge.handle.branch.unmanaged')._format(
|
||||
repository=r,
|
||||
branch=b,
|
||||
event=event,
|
||||
)
|
||||
_logger.info("Ignoring event %s on PR %s#%d for un-managed branch %s",
|
||||
event['action'], r, pr['number'], b)
|
||||
elif not branch.active:
|
||||
message = f"This PR targets the disabled branch {r}:{b}, it needs to be retargeted before it can be merged."
|
||||
message = env.ref('runbot_merge.handle.branch.inactive')._format(
|
||||
repository=r,
|
||||
branch=b,
|
||||
event=event,
|
||||
)
|
||||
if message and event['action'] not in ('synchronize', 'closed'):
|
||||
feedback(message=message)
|
||||
|
||||
@ -240,7 +248,7 @@ def handle_pr(env, event):
|
||||
if pr_obj.state == 'merged':
|
||||
feedback(
|
||||
close=True,
|
||||
message="@%s ya silly goose you can't reopen a merged PR." % event['sender']['login']
|
||||
message=env.ref('runbot_merge.handle.pr.merged')._format(event=event),
|
||||
)
|
||||
|
||||
if pr_obj.state == 'closed':
|
||||
|
||||
@ -0,0 +1,171 @@
|
||||
id,template,help
|
||||
runbot_merge.handle.branch.unmanaged,"This PR targets the un-managed branch {repository}:{branch}, it needs to be retargeted before it can be merged.","Notifies of event on PR whose branch is not managed by the mergebot.
|
||||
|
||||
repository: repository name
|
||||
branch: branch (ref) name
|
||||
event: complete pr event"
|
||||
runbot_merge.handle.branch.inactive,"This PR targets the disabled branch {repository}:{branch}, it needs to be retargeted before it can be merged.","Notifies of event on PR whose branch is deactivated.
|
||||
|
||||
repository: repository name
|
||||
branch: branch (ref) name
|
||||
event: complete pr event"
|
||||
runbot_merge.handle.pr.merged,@{event[sender][login]} ya silly goose you can't reopen a merged PR.,"Notifies that a user tried to reopen a merged PR.
|
||||
|
||||
Event: complete PR event"
|
||||
runbot_merge.pr.load.unmanaged,"Branch `{pr[base][ref]}` is not within my remit, imma just ignore it.","Notifies that a user tried to load a PR targeting a non-handled branch.
|
||||
|
||||
pr: pull request (github object)
|
||||
Repository: repository object (???)"
|
||||
runbot_merge.pr.load.fetched,"{pr.ping}I didn't know about this PR and had to retrieve its information, you may have to re-approve it as I didn't see previous commands.","Notifies that we did retrieve an unknown PR (either by request or as side effect of an interaction).
|
||||
|
||||
Pr: pr object we just created"
|
||||
runbot_merge.pr.branch.disabled,"{pr.ping}the target branch {pr.target.name!r} has been disabled, you may want to close this PR.","Notifies that the target branch for this PR was deactivated.
|
||||
|
||||
pr: pull request in question"
|
||||
runbot_merge.pr.merge.failed,{pr.ping}unable to stage: {reason},"Notifies that the PR could not be merged into the staging branch.
|
||||
|
||||
pr: pr object we tried to merge
|
||||
reason: error message
|
||||
exc: exception object"
|
||||
runbot_merge.pr.fetch.unmanaged,I'm sorry. Branch `{branch}` is not within my remit.,"Responds to a request to fetch a PR to an unmanaged branch.
|
||||
|
||||
repository: pr repository
|
||||
branch: target branch
|
||||
number: pr number"
|
||||
runbot_merge.command.access.no,"I'm sorry, @{user}. I'm afraid I can't do that.","Responds to command by a user who has no rights at all.
|
||||
|
||||
user: github login of comment sender
|
||||
pr: pr object to which the command was sent"
|
||||
runbot_merge.command.approve.failure,@{user} you may want to rebuild or fix this PR as it has failed CI.,"Responds to r+ of PR with failed CI.
|
||||
|
||||
user: github login of comment sender
|
||||
pr: pr object to which the command was sent"
|
||||
runbot_merge.command.unapprove.p0,"PR priority reset to 1, as pull requests with priority 0 ignore review state.","Responds to r- of pr in p=0.
|
||||
|
||||
user: github login of comment sender
|
||||
pr: pr object to which the command was sent"
|
||||
runbot_merge.command.method,Merge method set to {new_method}.,"Responds to the setting of the merge method.
|
||||
|
||||
new_method: ...
|
||||
pr: pr object to which the command was sent
|
||||
user: github login of the comment sender"
|
||||
runbot_merge.failure.approved,{pr.ping}{status!r} failed on this reviewed PR.,"Notification of failed status on a reviewed PR.
|
||||
|
||||
pr: pull request in question
|
||||
status: failed status"
|
||||
runbot_merge.pr.created,[Pull request status dashboard]({pr.url}).,"Initial comment on PR creation.
|
||||
|
||||
pr: created pr"
|
||||
runbot_merge.pr.linked.not_ready,{pr.ping}linked pull request(s) {siblings} not ready. Linked PRs are not staged until all of them are ready.,"Comment when a PR is ready (approved & validated) but it is linked to other PRs which are not.
|
||||
|
||||
pr: pr we're looking at
|
||||
siblings: its siblings, as a single comma-separated list of PR links"
|
||||
runbot_merge.pr.merge_method,"{pr.ping}because this PR has multiple commits, I need to know how to merge it:
|
||||
|
||||
{methods}","Comment when a PR is ready but doesn't have a merge method set
|
||||
|
||||
pr: the pr we can't stage
|
||||
methods: a markdown-formatted list of valid merge methods"
|
||||
runbot_merge.pr.staging.mismatch,"{pr.ping}we apparently missed updates to this PR and tried to stage it in a state which might not have been approved.
|
||||
|
||||
The properties {mismatch} were not correctly synchronized and have been updated.
|
||||
|
||||
<details><summary>differences</summary>
|
||||
|
||||
```diff
|
||||
{diff}```
|
||||
</details>
|
||||
|
||||
Note that we are unable to check the properties {unchecked}.
|
||||
|
||||
Please check and re-approve.
|
||||
","Comment when staging was attempted but a sanity check revealed the github state and the mergebot state differ.
|
||||
|
||||
pr: the pr we tried to stage
|
||||
mismatch: comma separated list of mismatched property names
|
||||
diff: patch-style view of the differing properties
|
||||
unchecked: comma-separated list of properties which can't be checked"
|
||||
runbot_merge.pr.staging.fail,{pr.ping}staging failed: {message},"Comment when a PR caused a staging to fail (normally only sent if the staging has a single batch, may be sent on multiple PRs depending whether the heuristic to guess the problematic PR of a batch succeeded)
|
||||
|
||||
pr: the pr
|
||||
message: staging failure information (error message, build link, etc...)"
|
||||
runbot_merge.forwardport.updates.closed,"{pr.ping}ancestor PR {parent.display_name} has been updated but this PR is {pr.state} and can't be updated to match.
|
||||
|
||||
You may want or need to manually update any followup PR.","Comment when a PR is updated and on of its followups is already merged or closed. Sent to the followup.
|
||||
|
||||
pr: the closed or merged PR
|
||||
parent: the modified ancestor PR"
|
||||
runbot_merge.forwardport.updates.conflict.parent,"{pr.ping}WARNING: the latest change ({pr.head}) triggered a conflict when updating the next forward-port ({next.display_name}), and has been ignored.
|
||||
|
||||
You will need to update this pull request differently, or fix the issue by hand on {next.display_name}.","Comment when a PR update triggers a conflict in a child.
|
||||
|
||||
pr: updated parent PR
|
||||
next: child PR in conflict"
|
||||
runbot_merge.forwardport.updates.conflict.child,"{pr.ping}WARNING: the update of {previous.display_name} to {previous.head} has caused a conflict in this pull request, data may have been lost.{stdout}{stderr}","Comment when a PR update followup is in conflict.
|
||||
|
||||
pr: PR where update followup conflict happened
|
||||
previous: parent PR which triggered the followup
|
||||
stdout: markdown-formatted stdout of git, if any
|
||||
stderr: markdown-formatted stderr of git, if any"
|
||||
runbot_merge.forwardport.update.detached,{pr.ping}this PR was modified / updated and has become a normal PR. It should be merged the normal way (via @{pr.repository.project_id.github_prefix}),"Comment when a forwardport PR gets updated, documents that the PR now needs to be merged the “normal” way.
|
||||
|
||||
pr: the pr in question "
|
||||
runbot_merge.forwardport.reopen.detached,{pr.ping}this PR was closed then reopened. It should be merged the normal way (via @{pr.repository.project_id.github_prefix}),"Comment when a forwardport PR gets closed then reopened, documents that the PR is now in a detached state.
|
||||
|
||||
pr: the pr in question"
|
||||
runbot_merge.forwardport.ci.failed,{pr.ping}{ci} failed on this forward-port PR,"Comment when CI fails on a forward-port PR (which thus won't port any further, for now).
|
||||
|
||||
pr: the pr in question
|
||||
ci: the failed status"
|
||||
runbot_merge.forwardport.failure.discrepancy,{pr.ping}this pull request can not be forward-ported: next branch is {next!r} but linked pull request {linked.display_name} has a next branch {other!r}.,"Comment when we tried to forward port a PR batch, but the PRs have different next targets (unlikely to happen really).
|
||||
|
||||
pr: the pr we tried to forward port
|
||||
linked: the linked PR with a different next target
|
||||
next: next target for the current pr
|
||||
other: next target for the other pr"
|
||||
runbot_merge.forwardport.failure.conflict,"{pr.ping}the next pull request ({new.display_name}) is in conflict. You can merge the chain up to here by saying
|
||||
> @{pr.repository.project_id.fp_github_name} r+
|
||||
{footer}","Comment when a forward port was created but is in conflict, warns of that & gives instructions for current PR.
|
||||
|
||||
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
|
||||
|
||||
pr: the original (source) PR
|
||||
outstanding: markdown-formatted list of outstanding PRs"
|
||||
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?).
|
||||
|
||||
In the former case, you may want to edit this PR message as well.
|
||||
{footer}","Comment when a forward-port failed.
|
||||
|
||||
pr: the new pr (in failure)
|
||||
commits: markdown-formatted list of source commits, indicating which failed
|
||||
stdout: git's stdout
|
||||
stderr: git's stderr
|
||||
footer: some footer text"
|
||||
runbot_merge.forwardport.linked,"{pr.ping}while this was properly forward-ported, at least one co-dependent PR ({siblings}) did not succeed. You will need to fix it before this can be merged.
|
||||
|
||||
Both this PR and the others will need to be approved via `@{pr.repository.project_id.github_prefix} r+` as they are all considered “in conflict”.
|
||||
{footer} ","Comment when a forward port succeeded but at least one sibling failed.
|
||||
|
||||
pr: the current pr (new)
|
||||
siblings: comma-separated list of sibling links
|
||||
footer: some footer text"
|
||||
runbot_merge.forwardport.final,"{pr.ping}this PR targets {pr.target.name} and is the last of the forward-port chain{containing}
|
||||
{ancestors}
|
||||
To merge the full chain, use
|
||||
> @{pr.repository.project_id.fp_github_name} r+
|
||||
{footer}","Comment when a forward port was created and is the last of a sequence (target the limit branch).
|
||||
|
||||
pr: the new forward port
|
||||
containing: label changing depending whether there are ancestors to merge
|
||||
ancestors: markdown formatted list of parent PRs which can be approved as part of the chain
|
||||
footer: a footer"
|
||||
runbot_merge.forwardport.intermediate,"This PR targets {pr.target.name} and is part of the forward-port chain. Further PRs will be created up to {pr.limit_id.name}.
|
||||
{footer}","Comment when a forward port was succcessfully created but is not the last of the line.
|
||||
|
||||
pr: the new forward port
|
||||
footer: a footer"
|
||||
|
@ -16,6 +16,7 @@ import time
|
||||
|
||||
from difflib import Differ
|
||||
from itertools import takewhile
|
||||
from typing import Optional
|
||||
|
||||
import requests
|
||||
import werkzeug
|
||||
@ -111,15 +112,17 @@ All substitutions are tentatively applied sequentially to the input.
|
||||
# fetch PR object and handle as *opened*
|
||||
issue, pr = gh.pr(number)
|
||||
|
||||
feedback = self.env['runbot_merge.pull_requests.feedback'].create
|
||||
if not self.project_id._has_branch(pr['base']['ref']):
|
||||
_logger.info("Tasked with loading PR %d for un-managed branch %s:%s, ignoring",
|
||||
number, self.name, pr['base']['ref'])
|
||||
feedback({
|
||||
'repository': self.id,
|
||||
'pull_request': number,
|
||||
'message': "Branch `{}` is not within my remit, imma just ignore it.".format(pr['base']['ref']),
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.load.unmanaged')._send(
|
||||
repository=self,
|
||||
pull_request=number,
|
||||
format_args = {
|
||||
'pr': pr,
|
||||
'repository': self,
|
||||
},
|
||||
)
|
||||
return
|
||||
|
||||
# if the PR is already loaded, force sync a few attributes
|
||||
@ -150,20 +153,13 @@ All substitutions are tentatively applied sequentially to the input.
|
||||
'pull_request': pr,
|
||||
'sender': {'login': self.project_id.github_prefix}
|
||||
}) + '. '
|
||||
feedback({
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr_id.repository.id,
|
||||
'pull_request': number,
|
||||
'message': f"{edit}. {edit2}{sync}.",
|
||||
})
|
||||
return
|
||||
|
||||
feedback({
|
||||
'repository': self.id,
|
||||
'pull_request': number,
|
||||
'message': "%sI didn't know about this PR and had to retrieve "
|
||||
"its information, you may have to re-approve it as "
|
||||
"I didn't see previous commands." % pr_id.ping()
|
||||
})
|
||||
sender = {'login': self.project_id.github_prefix}
|
||||
# init the PR to the null commit so we can later synchronise it back
|
||||
# back to the "proper" head while resetting reviews
|
||||
@ -212,14 +208,21 @@ All substitutions are tentatively applied sequentially to the input.
|
||||
'pull_request': pr,
|
||||
'sender': sender,
|
||||
})
|
||||
pr_id = self.env['runbot_merge.pull_requests'].search([
|
||||
('repository.name', '=', pr['base']['repo']['full_name']),
|
||||
('number', '=', number),
|
||||
])
|
||||
if pr['state'] == 'closed':
|
||||
# don't go through controller because try_closing does weird things
|
||||
# for safety / race condition reasons which ends up committing
|
||||
# and breaks everything
|
||||
self.env['runbot_merge.pull_requests'].search([
|
||||
('repository.name', '=', pr['base']['repo']['full_name']),
|
||||
('number', '=', number),
|
||||
]).state = 'closed'
|
||||
pr_id.state = 'closed'
|
||||
|
||||
self.env.ref('runbot_merge.pr.load.fetched')._send(
|
||||
repository=self,
|
||||
pull_request=number,
|
||||
format_args={'pr': pr_id},
|
||||
)
|
||||
|
||||
def having_branch(self, branch):
|
||||
branches = self.env['runbot_merge.branch'].search
|
||||
@ -281,10 +284,11 @@ class Branch(models.Model):
|
||||
"Target branch deactivated by %r.",
|
||||
self.env.user.login,
|
||||
)
|
||||
tmpl = self.env.ref('runbot_merge.pr.branch.disabled')
|
||||
self.env['runbot_merge.pull_requests.feedback'].create([{
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': f'Hey {pr.ping()}the target branch {pr.target.name!r} has been disabled, you may want to close this PR.',
|
||||
'message': tmpl._format(pr=pr),
|
||||
} for pr in self.prs])
|
||||
return True
|
||||
|
||||
@ -392,11 +396,11 @@ class Branch(models.Model):
|
||||
reason = json.loads(str(reason))['message'].lower()
|
||||
|
||||
pr.state = 'error'
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': f'{pr.ping()}unable to stage: {reason}',
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.merge.failed')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
format_args= {'pr': pr, 'reason': reason, 'exc': e},
|
||||
)
|
||||
else:
|
||||
first = False
|
||||
|
||||
@ -584,16 +588,16 @@ class PullRequests(models.Model):
|
||||
repo_name = fields.Char(related='repository.name')
|
||||
message_title = fields.Char(compute='_compute_message_title')
|
||||
|
||||
def ping(self, author=True, reviewer=True):
|
||||
P = self.env['res.partner']
|
||||
s = ' '.join(
|
||||
f'@{p.github_login}'
|
||||
for p in (self.author if author else P) | (self.reviewed_by if reviewer else P)
|
||||
if p
|
||||
)
|
||||
if s:
|
||||
s += ' '
|
||||
return s
|
||||
ping = fields.Char(compute='_compute_ping')
|
||||
|
||||
@api.depends('author.github_login', 'reviewed_by.github_login')
|
||||
def _compute_ping(self):
|
||||
for pr in self:
|
||||
s = ' '.join(
|
||||
f'@{p.github_login}'
|
||||
for p in (pr.author | pr.reviewed_by )
|
||||
)
|
||||
pr.ping = s and (s + ' ')
|
||||
|
||||
@api.depends('repository.name', 'number')
|
||||
def _compute_url(self):
|
||||
@ -739,11 +743,11 @@ class PullRequests(models.Model):
|
||||
return
|
||||
|
||||
if target and not repo.project_id._has_branch(target):
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': repo.id,
|
||||
'pull_request': number,
|
||||
'message': "I'm sorry. Branch `{}` is not within my remit.".format(target),
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.fetch.unmanaged')._send(
|
||||
repository=repo,
|
||||
pull_request=number,
|
||||
format_args={'repository': repo, 'branch': target, 'number': number}
|
||||
)
|
||||
return
|
||||
|
||||
pr = self.search([
|
||||
@ -825,16 +829,15 @@ class PullRequests(models.Model):
|
||||
)
|
||||
return 'ok'
|
||||
|
||||
Feedback = self.env['runbot_merge.pull_requests.feedback']
|
||||
if not (is_author or any(cmd == 'override' for cmd, _ in commands)):
|
||||
# no point even parsing commands
|
||||
_logger.info("ignoring comment of %s (%s): no ACL to %s",
|
||||
login, name, self.display_name)
|
||||
Feedback.create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message': "I'm sorry, @{}. I'm afraid I can't do that.".format(login)
|
||||
})
|
||||
self.env.ref('runbot_merge.command.access.no')._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
format_args={'user': login, 'pr': self}
|
||||
)
|
||||
return 'ignored'
|
||||
|
||||
applied, ignored = [], []
|
||||
@ -890,11 +893,11 @@ class PullRequests(models.Model):
|
||||
if self.status == 'failure':
|
||||
# the normal infrastructure is for failure and
|
||||
# prefixes messages with "I'm sorry"
|
||||
Feedback.create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message': "@{} you may want to rebuild or fix this PR as it has failed CI.".format(login),
|
||||
})
|
||||
self.env.ref("runbot_merge.command.approve.failure")._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
format_args={'user': login, 'pr': self},
|
||||
)
|
||||
elif not param and is_author:
|
||||
newstate = RMINUS.get(self.state)
|
||||
if self.priority == 0 or newstate:
|
||||
@ -902,11 +905,11 @@ class PullRequests(models.Model):
|
||||
self.state = newstate
|
||||
if self.priority == 0:
|
||||
self.priority = 1
|
||||
Feedback.create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message': "PR priority reset to 1, as pull requests with priority 0 ignore review state.",
|
||||
})
|
||||
self.env.ref("runbot_merge.command.unapprove.p0")._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
format_args={'user': login, 'pr': self},
|
||||
)
|
||||
self.unstage("unreviewed (r-) by %s", login)
|
||||
ok = True
|
||||
else:
|
||||
@ -938,11 +941,11 @@ class PullRequests(models.Model):
|
||||
self.merge_method = param
|
||||
ok = True
|
||||
explanation = next(label for value, label in type(self).merge_method.selection if value == param)
|
||||
Feedback.create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message':"Merge method set to %s." % explanation
|
||||
})
|
||||
self.env.ref("runbot_merge.command.method")._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
format_args={'new_method': explanation, 'pr': self, 'user': login},
|
||||
)
|
||||
elif command == 'override':
|
||||
overridable = author.override_rights\
|
||||
.filtered(lambda r: not r.repository_id or (r.repository_id == self.repository))\
|
||||
@ -983,7 +986,7 @@ class PullRequests(models.Model):
|
||||
if msgs:
|
||||
joiner = ' ' if len(msgs) == 1 else '\n- '
|
||||
msgs.insert(0, "I'm sorry, @{}:".format(login))
|
||||
Feedback.create({
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message': joiner.join(msgs),
|
||||
@ -1078,11 +1081,11 @@ class PullRequests(models.Model):
|
||||
def _notify_ci_failed(self, ci):
|
||||
# only report an issue of the PR is already approved (r+'d)
|
||||
if self.state == 'approved':
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': self.repository.id,
|
||||
'pull_request': self.number,
|
||||
'message': "%s%r failed on this reviewed PR." % (self.ping(), ci),
|
||||
})
|
||||
self.env.ref("runbot_merge.failure.approved")._send(
|
||||
repository=self.repository,
|
||||
pull_request=self.number,
|
||||
format_args={'pr': self, 'status': ci}
|
||||
)
|
||||
|
||||
def _auto_init(self):
|
||||
super(PullRequests, self)._auto_init()
|
||||
@ -1108,11 +1111,11 @@ class PullRequests(models.Model):
|
||||
pr._validate(json.loads(c.statuses or '{}'))
|
||||
|
||||
if pr.state not in ('closed', 'merged'):
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': f"[Pull request status dashboard]({pr.url}).",
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.created')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
format_args={'pr': pr},
|
||||
)
|
||||
return pr
|
||||
|
||||
def _from_gh(self, description, author=None, branch=None, repo=None):
|
||||
@ -1211,14 +1214,14 @@ class PullRequests(models.Model):
|
||||
unready = (prs - ready).sorted(key=lambda p: (p.repository.name, p.number))
|
||||
|
||||
for r in ready:
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': r.repository.id,
|
||||
'pull_request': r.number,
|
||||
'message': "{}linked pull request(s) {} not ready. Linked PRs are not staged until all of them are ready.".format(
|
||||
r.ping(),
|
||||
', '.join(map('{0.display_name}'.format, unready))
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.linked.not_ready')._send(
|
||||
repository=r.repository,
|
||||
pull_request=r.number,
|
||||
format_args={
|
||||
'pr': r,
|
||||
'siblings': ', '.join(map('{0.display_name}'.format, unready))
|
||||
},
|
||||
)
|
||||
r.link_warned = True
|
||||
if commit:
|
||||
self.env.cr.commit()
|
||||
@ -1236,14 +1239,11 @@ class PullRequests(models.Model):
|
||||
('merge_method', '=', False),
|
||||
('method_warned', '=', False),
|
||||
]):
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': r.repository.id,
|
||||
'pull_request': r.number,
|
||||
'message': "%sbecause this PR has multiple commits, I need to know how to merge it:\n\n%s" % (
|
||||
r.ping(),
|
||||
methods,
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.merge_method')._send(
|
||||
repository=r.repository,
|
||||
pull_request=r.number,
|
||||
format_args={'pr': r, 'methods':methods},
|
||||
)
|
||||
r.method_warned = True
|
||||
if commit:
|
||||
self.env.cr.commit()
|
||||
@ -1663,6 +1663,33 @@ class Feedback(models.Model):
|
||||
to_remove.append(f.id)
|
||||
self.browse(to_remove).unlink()
|
||||
|
||||
class FeedbackTemplate(models.Model):
|
||||
_name = 'runbot_merge.pull_requests.feedback.template'
|
||||
_description = "str.format templates for feedback messages, no integration," \
|
||||
"but that's their purpose"
|
||||
_inherit = ['mail.thread']
|
||||
|
||||
template = fields.Text(tracking=True)
|
||||
help = fields.Text(readonly=True)
|
||||
|
||||
def _format(self, **args):
|
||||
return self.template.format_map(args)
|
||||
|
||||
def _send(self, *, repository: Repository, pull_request: int, format_args: dict, token_field: Optional[str] = None) -> Optional[Feedback]:
|
||||
try:
|
||||
feedback = {
|
||||
'repository': repository.id,
|
||||
'pull_request': pull_request,
|
||||
'message': self.template.format_map(format_args),
|
||||
}
|
||||
if token_field:
|
||||
feedback['token_field'] = token_field
|
||||
return self.env['runbot_merge.pull_requests.feedback'].create(feedback)
|
||||
except Exception:
|
||||
_logger.exception("Failed to render template %s", self.get_external_id())
|
||||
raise
|
||||
return None
|
||||
|
||||
class Commit(models.Model):
|
||||
"""Represents a commit onto which statuses might be posted,
|
||||
independent of everything else as commits can be created by
|
||||
@ -1900,11 +1927,11 @@ class Stagings(models.Model):
|
||||
prs = prs or self.batch_ids.prs
|
||||
prs.write({'state': 'error'})
|
||||
for pr in prs:
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': "%sstaging failed: %s" % (pr.ping(), message),
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.staging.fail')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
format_args={'pr': pr, 'message': message},
|
||||
)
|
||||
|
||||
self.batch_ids.write({'active': False})
|
||||
self.write({
|
||||
@ -2213,31 +2240,16 @@ class Batch(models.Model):
|
||||
"data mismatch on %s:\n%s",
|
||||
pr.display_name, diff
|
||||
)
|
||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||
'repository': pr.repository.id,
|
||||
'pull_request': pr.number,
|
||||
'message': """\
|
||||
{ping}we apparently missed updates to this PR and tried to stage it in a state \
|
||||
which might not have been approved.
|
||||
|
||||
The properties {mismatch} were not correctly synchronized and have been updated.
|
||||
|
||||
<details><summary>differences</summary>
|
||||
|
||||
```diff
|
||||
{diff}```
|
||||
</details>
|
||||
|
||||
Note that we are unable to check the properties {unchecked}.
|
||||
|
||||
Please check and re-approve.
|
||||
""".format(
|
||||
ping=pr.ping(),
|
||||
mismatch=', '.join(pr_fields[f].string for f in e.args[0]),
|
||||
diff=diff,
|
||||
unchecked=', '.join(pr_fields[f].string for f in UNCHECKABLE),
|
||||
)
|
||||
})
|
||||
self.env.ref('runbot_merge.pr.staging.mismatch')._send(
|
||||
repository=pr.repository,
|
||||
pull_request=pr.number,
|
||||
format_args={
|
||||
'pr': pr,
|
||||
'mismatch': ', '.join(pr_fields[f].string for f in e.args[0]),
|
||||
'diff': diff,
|
||||
'unchecked': ', '.join(pr_fields[f].string for f in UNCHECKABLE)
|
||||
}
|
||||
)
|
||||
return self.env['runbot_merge.batch']
|
||||
|
||||
# update meta to new heads
|
||||
|
||||
@ -25,3 +25,4 @@ access_runbot_merge_pull_requests,User access to PR,model_runbot_merge_pull_requ
|
||||
access_runbot_merge_pull_requests_feedback,Users have no reason to access feedback,model_runbot_merge_pull_requests_feedback,,0,0,0,0
|
||||
access_runbot_merge_review_rights_2,Users can see partners,model_res_partner_review,base.group_user,1,0,0,0
|
||||
access_runbot_merge_review_override_2,Users can see partners,model_res_partner_override,base.group_user,1,0,0,0
|
||||
runbot_merge.access_runbot_merge_pull_requests_feedback_template,access_runbot_merge_pull_requests_feedback_template,runbot_merge.model_runbot_merge_pull_requests_feedback_template,base.group_system,1,1,0,0
|
||||
|
||||
|
@ -3212,10 +3212,10 @@ class TestUnknownPR:
|
||||
seen(env, prx, users),
|
||||
(users['reviewer'], 'hansen r+'),
|
||||
(users['reviewer'], 'hansen r+'),
|
||||
(users['user'], "I didn't know about this PR and had to "
|
||||
seen(env, prx, users),
|
||||
(users['user'], f"@{users['user']} @{users['reviewer']} I didn't know about this PR and had to "
|
||||
"retrieve its information, you may have to "
|
||||
"re-approve it as I didn't see previous commands."),
|
||||
seen(env, prx, users),
|
||||
]
|
||||
|
||||
pr = env['runbot_merge.pull_requests'].search([
|
||||
@ -3265,10 +3265,13 @@ class TestUnknownPR:
|
||||
assert pr.comments == [
|
||||
seen(env, pr, users),
|
||||
(users['reviewer'], 'hansen r+'),
|
||||
(users['user'], "I didn't know about this PR and had to retrieve "
|
||||
seen(env, pr, users),
|
||||
# reviewer is set because fetch replays all the comments (thus
|
||||
# setting r+ and reviewer) but then syncs the head commit thus
|
||||
# unsetting r+ but leaving the reviewer
|
||||
(users['user'], f"@{users['user']} @{users['reviewer']} I didn't know about this PR and had to retrieve "
|
||||
"its information, you may have to re-approve it "
|
||||
"as I didn't see previous commands."),
|
||||
seen(env, pr, users),
|
||||
]
|
||||
|
||||
def test_rplus_unmanaged(self, env, repo, users, config):
|
||||
|
||||
@ -63,7 +63,7 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf
|
||||
assert pr.comments == [
|
||||
(users['reviewer'], "hansen r+"),
|
||||
seen(env, pr, users),
|
||||
(users['user'], "Hey @%(user)s @%(reviewer)s the target branch 'other' has been disabled, you may want to close this PR." % users),
|
||||
(users['user'], "@%(user)s @%(reviewer)s the target branch 'other' has been disabled, you may want to close this PR." % users),
|
||||
]
|
||||
|
||||
with repo:
|
||||
|
||||
@ -33,11 +33,42 @@
|
||||
</field>
|
||||
</record>
|
||||
|
||||
<menuitem name="Configuration" id="menu_configuration" parent="runbot_merge_menu"/>
|
||||
<record id="action_feedback" model="ir.actions.act_window">
|
||||
<field name="name">Feedback Templates tree</field>
|
||||
<field name="res_model">runbot_merge.pull_requests.feedback.template</field>
|
||||
</record>
|
||||
<record id="tree_feedback" model="ir.ui.view">
|
||||
<field name="name">Feedback Templates</field>
|
||||
<field name="model">runbot_merge.pull_requests.feedback.template</field>
|
||||
<field name="arch" type="xml">
|
||||
<tree>
|
||||
<field name="template"/>
|
||||
<field name="help"/>
|
||||
</tree>
|
||||
</field>
|
||||
</record>
|
||||
<record id="form_feedback" model="ir.ui.view">
|
||||
<field name="name">Feedback Templates form</field>
|
||||
<field name="model">runbot_merge.pull_requests.feedback.template</field>
|
||||
<field name="arch" type="xml">
|
||||
<form>
|
||||
<sheet>
|
||||
<field name="help"/>
|
||||
<field name="template"/>
|
||||
</sheet>
|
||||
<div class="oe_chatter">
|
||||
<field name="message_ids"/>
|
||||
</div>
|
||||
</form>
|
||||
</field>
|
||||
</record>
|
||||
|
||||
<menuitem name="Configuration" id="menu_configuration" parent="runbot_merge_menu">
|
||||
<menuitem name="CI Overrides" id="menu_configuration_overrides"
|
||||
parent="menu_configuration"
|
||||
action="action_overrides"/>
|
||||
<menuitem name="Review Rights" id="menu_configuration_review"
|
||||
parent="menu_configuration"
|
||||
action="action_review"/>
|
||||
<menuitem name="Feedback Templates" id="menu_configuration_feedback"
|
||||
action="action_feedback"/>
|
||||
</menuitem>
|
||||
</odoo>
|
||||
|
||||
Loading…
Reference in New Issue
Block a user