[IMP] *: add fw=no, deprecate ignore

Without fw-bot being its bearer, "ignore" is a lot less clear than it
used to as it looks to be asking to ignore the PR entirely (as if it
was targeted to an unmanaged branch).

Deprecate this command, and tack on the shortcut to the fw
subcommand. It is slightly sub-par as technically it does not quite
fit with the other subcommands, and furthermore can't be disabled via
fw=default... although maybe it could be? Maybe instead of setting the
limit fw=no could set that value to the forwardport mode, and the
fw_policy users could check that? It would require some more finessing
tho:

- `DEFAULT` would need to be accessible to the author as well as the
  reviewers so the author could toggle between `NO` and `DEFAULT`.
- There should probably be a warning of some sort when setting a limit
  to an unportable PR.
- The dashboards would need to take `NO` in account (though I guess
  that's just defaulting the limit to the target).
This commit is contained in:
Xavier Morel 2024-06-12 16:08:25 +02:00
parent 413027ad5b
commit 7711d09854
3 changed files with 38 additions and 8 deletions

View File

@ -45,29 +45,39 @@ def test_configure_fp_limit(env, config, make_repo, source, limit, count, port):
r = requests.get(f'http://localhost:{port}/{prod.name}/pull/{pr.number}.png')
assert r.ok
def test_ignore(env, config, make_repo):
def test_ignore(env, config, make_repo, users):
""" Provide an "ignore" command which is equivalent to setting the limit
to target
"""
prod, other = make_basic(env, config, make_repo)
prod, _ = make_basic(env, config, make_repo, statuses="default")
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
with prod:
[c] = prod.make_commits('a', Commit('c', tree={'0': '0'}), ref='heads/mybranch')
pr = prod.make_pr(target='a', head='mybranch')
prod.post_status(c, 'success', 'legal/cla')
prod.post_status(c, 'success', 'ci/runbot')
pr.post_comment('hansen r+ ignore', config['role_reviewer']['token'])
prod.post_status(c, 'success')
env.run_crons()
with prod:
pr.post_comment('hansen ignore', config['role_reviewer']['token'])
pr.post_comment('hansen r+ fw=no', config['role_reviewer']['token'])
env.run_crons()
pr_id = env['runbot_merge.pull_requests'].search([('number', '=', pr.number)])
assert pr_id.limit_id == branch_a
with prod:
prod.post_status('staging.a', 'success', 'legal/cla')
prod.post_status('staging.a', 'success', 'ci/runbot')
prod.post_status('staging.a', 'success')
env.run_crons()
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
"should not have created a forward port"
assert pr.comments == [
seen(env, pr, 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."),
]
@pytest.mark.expect_log_errors(reason="one of the limit-setting does not provide a branch name")
def test_disable(env, config, make_repo, users):

View File

@ -137,6 +137,7 @@ class SkipChecks:
class FW(enum.Enum):
DEFAULT = enum.auto()
NO = enum.auto()
SKIPCI = enum.auto()
SKIPMERGE = enum.auto()

View File

@ -812,7 +812,24 @@ class PullRequests(models.Model):
case commands.Close() if source_author:
feedback(close=True)
case commands.FW():
if source_reviewer or is_reviewer:
if command == commands.FW.NO:
if is_author:
for p in self.batch_id.prs:
ping, m = p._maybe_update_limit(self.target.name)
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:
(self.source_id or self).batch_id.fw_policy = command.name.lower()
match command:
case commands.FW.DEFAULT:
@ -825,6 +842,8 @@ class PullRequests(models.Model):
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.")
limit = branch or self.target.name
for p in self.batch_id.prs:
ping, m = p._maybe_update_limit(limit)