[IMP] *: convert fw=no to a genuine forward-porting policy

After seeing it be used, I foresee confusion around the current
behaviour (where it sets the limit), as one would expect the `fw=`
flags to affect one another when it looks like that would make sense
e.g. no/default/skipci/skipmerge all specify how to forward port, so
`fw=default` not doing anything after you've said `fw=no` (possibly by
mistake) would be fucking weird.

Also since the author can set limits, allow them to reset the fw
policy to default (keep skipci for reviewers), and for @d-fence add a
`fw=disabled` alias.

Fixes #902
This commit is contained in:
Xavier Morel 2024-06-28 16:03:03 +02:00
parent 0206d5f977
commit 94cf3e9647
5 changed files with 23 additions and 31 deletions

View File

@ -550,6 +550,9 @@ class Stagings(models.Model):
if vals.get('active') is False and self.state == 'success':
# check all batches to see if they should be forward ported
for b in self.with_context(active_test=False).batch_ids:
if b.fw_policy == 'no':
continue
# if all PRs of a batch have parents they're part of an FP
# sequence and thus handled separately, otherwise they're
# considered regular merges

View File

@ -72,8 +72,8 @@ def test_ignore(env, config, make_repo, users):
(users['reviewer'], "hansen ignore"),
(users['reviewer'], "hansen r+ fw=no"),
(users['user'], "'ignore' is deprecated, use 'fw=no' to disable forward porting."),
(users['user'], "Forward-port disabled."),
(users['user'], "Forward-port disabled."),
(users['user'], "Forward-port disabled (via limit)."),
(users['user'], "Disabled forward-porting."),
]
@pytest.mark.expect_log_errors(reason="one of the limit-setting does not provide a branch name")

View File

@ -68,9 +68,10 @@ class Batch(models.Model):
active = fields.Boolean(compute='_compute_active', store=True, help="closed batches (batches containing only closed PRs)")
fw_policy = fields.Selection([
('no', "Do not port forward"),
('default', "Default"),
('skipci', "Skip CI"),
], required=True, default="default", string="Forward Port Policy")
], required=True, default="default", string="Forward Port Policy", tracking=True)
merge_date = fields.Datetime(tracking=True)
# having skipchecks skip both validation *and approval* makes sense because
@ -89,7 +90,7 @@ class Batch(models.Model):
('default', "Default"),
('priority', "Priority"),
('alone', "Alone"),
], default='default', group_operator=None, required=True,
], default='default', group_operator=None, required=True, tracking=True,
column_type=enum(_name, 'priority'),
)

View File

@ -363,6 +363,8 @@ class Parser:
self.assert_next('=')
f = next(self.it, "")
try:
if f in ('disable', 'disabled'):
return FW.NO
return FW[f.upper()]
except KeyError:
raise CommandError(f"unknown fw configuration {f or None!r}") from None

View File

@ -873,35 +873,21 @@ For your own safety I've ignored *everything in your entire comment*.
case commands.Close() if source_author:
feedback(close=True)
case commands.FW():
if command == commands.FW.NO:
if is_author:
for p in self.batch_id.prs:
ping, m = p._maybe_update_limit(self.target.name)
match command:
case commands.FW.NO if is_author or source_author:
message = "Disabled forward-porting."
case commands.FW.DEFAULT if is_author or source_author:
message = "Waiting for CI to create followup forward-ports."
case commands.FW.SKIPCI if is_reviewer or source_reviewer:
message = "Not waiting for CI to create followup forward-ports."
case commands.FW.SKIPMERGE if is_reviewer or source_reviewer:
message = "Not waiting for merge to create followup forward-ports."
case _:
msg = f"you don't have the right to {command}."
if ping and p == self:
msg = m
else:
if ping:
m = f"@{login} {m}"
self.env['runbot_merge.pull_requests.feedback'].create({
'repository': p.repository.id,
'pull_request': p.number,
'message': m,
})
else:
msg = "you can't set a forward-port limit."
elif source_reviewer or is_reviewer:
if not msg:
(self.source_id or self).batch_id.fw_policy = command.name.lower()
match command:
case commands.FW.DEFAULT:
message = "Waiting for CI to create followup forward-ports."
case commands.FW.SKIPCI:
message = "Not waiting for CI to create followup forward-ports."
case commands.FW.SKIPMERGE:
message = "Not waiting for merge to create followup forward-ports."
feedback(message=message)
else:
msg = "you can't configure forward-port CI."
case commands.Limit(branch) if is_author:
if branch is None:
feedback(message="'ignore' is deprecated, use 'fw=no' to disable forward porting.")
@ -957,7 +943,7 @@ For your own safety I've ignored *everything in your entire comment*.
if not self.source_id and self.state != 'merged':
self.limit_id = limit_id
if branch_key(limit_id) <= branch_key(self.target):
return False, "Forward-port disabled."
return False, "Forward-port disabled (via limit)."
else:
return False, f"Forward-porting to {limit_id.name!r}."