mirror of
https://github.com/odoo/runbot.git
synced 2025-03-27 13:25:47 +07:00
[IMP] forwardport: improve disable fp UI
* always allow specifying the PR's own branch as a forward-port limit / target (even if deactivated or disabled) * add an "ignore" alias to "up to <pr target>" for clarity * add dedicated feedback when deactivating forward-port of a PR Fixes #191
This commit is contained in:
parent
66d65ba550
commit
6d4923928a
@ -188,7 +188,15 @@ class PullRequests(models.Model):
|
|||||||
|
|
||||||
# TODO: don't use a mutable tokens iterator
|
# TODO: don't use a mutable tokens iterator
|
||||||
tokens = iter(tokens)
|
tokens = iter(tokens)
|
||||||
for token in tokens:
|
while True:
|
||||||
|
token = next(tokens, None)
|
||||||
|
if token is None:
|
||||||
|
break
|
||||||
|
|
||||||
|
if token == 'ignore': # replace 'ignore' by 'up to <pr_branch>'
|
||||||
|
token = 'up'
|
||||||
|
tokens = itertools.chain(['to', self.target.name], tokens)
|
||||||
|
|
||||||
if token in ('r+', 'review+') and self.source_id._pr_acl(author).is_reviewer:
|
if token in ('r+', 'review+') and self.source_id._pr_acl(author).is_reviewer:
|
||||||
# don't update the root ever
|
# don't update the root ever
|
||||||
for pr in filter(lambda p: p.parent_id, self._iter_ancestors()):
|
for pr in filter(lambda p: p.parent_id, self._iter_ancestors()):
|
||||||
@ -208,12 +216,16 @@ class PullRequests(models.Model):
|
|||||||
])
|
])
|
||||||
if not limit_id:
|
if not limit_id:
|
||||||
msg = "There is no branch %r, it can't be used as a forward port target." % limit
|
msg = "There is no branch %r, it can't be used as a forward port target." % limit
|
||||||
|
elif limit_id == self.target:
|
||||||
|
msg = "Forward-port disabled."
|
||||||
|
self.limit_id = limit_id
|
||||||
elif not limit_id.fp_enabled:
|
elif not limit_id.fp_enabled:
|
||||||
msg = "Branch %r is disabled, it can't be used as a forward port target." % limit_id.name
|
msg = "Branch %r is disabled, it can't be used as a forward port target." % limit_id.name
|
||||||
else:
|
else:
|
||||||
msg = "Forward-porting to %r." % limit_id.name
|
msg = "Forward-porting to %r." % limit_id.name
|
||||||
self.limit_id = limit_id
|
self.limit_id = limit_id
|
||||||
|
|
||||||
|
_logger.info("%s: %s", author, msg)
|
||||||
self.env['runbot_merge.pull_requests.feedback'].create({
|
self.env['runbot_merge.pull_requests.feedback'].create({
|
||||||
'repository': self.repository.id,
|
'repository': self.repository.id,
|
||||||
'pull_request': self.number,
|
'pull_request': self.number,
|
||||||
@ -221,6 +233,7 @@ class PullRequests(models.Model):
|
|||||||
'token_field': 'fp_github_token',
|
'token_field': 'fp_github_token',
|
||||||
})
|
})
|
||||||
|
|
||||||
|
|
||||||
def _validate(self, statuses):
|
def _validate(self, statuses):
|
||||||
_logger = logging.getLogger(__name__).getChild('forwardport.next')
|
_logger = logging.getLogger(__name__).getChild('forwardport.next')
|
||||||
failed = super()._validate(statuses)
|
failed = super()._validate(statuses)
|
||||||
|
@ -761,6 +761,54 @@ def test_limit_configure(env, config, make_repo):
|
|||||||
assert prs[1].number == originals[1]
|
assert prs[1].number == originals[1]
|
||||||
assert prs[2].number == originals[2]
|
assert prs[2].number == originals[2]
|
||||||
|
|
||||||
|
def test_limit_self_disabled(env, config, make_repo):
|
||||||
|
""" Allow setting target as limit even if it's disabled
|
||||||
|
"""
|
||||||
|
prod, other = make_basic(env, config, make_repo)
|
||||||
|
bot_name = env['runbot_merge.project'].search([]).fp_github_name
|
||||||
|
branch_a = env['runbot_merge.branch'].search([('name', '=', 'a')])
|
||||||
|
branch_a.fp_target = False
|
||||||
|
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+\n%s up to a' % bot_name, 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')
|
||||||
|
|
||||||
|
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
|
||||||
|
"should not have created a forward port"
|
||||||
|
|
||||||
|
def test_fp_ignore(env, config, make_repo):
|
||||||
|
""" Provide an "ignore" command which is equivalent to setting the limit
|
||||||
|
to target
|
||||||
|
"""
|
||||||
|
prod, other = make_basic(env, config, make_repo)
|
||||||
|
bot_name = env['runbot_merge.project'].search([]).fp_github_name
|
||||||
|
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+\n%s ignore' % bot_name, 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')
|
||||||
|
|
||||||
|
assert env['runbot_merge.pull_requests'].search([]) == pr_id,\
|
||||||
|
"should not have created a forward port"
|
||||||
|
|
||||||
@pytest.mark.parametrize('enabled', ['active', 'fp_target'])
|
@pytest.mark.parametrize('enabled', ['active', 'fp_target'])
|
||||||
def test_limit_disable(env, config, make_repo, users, enabled):
|
def test_limit_disable(env, config, make_repo, users, enabled):
|
||||||
""" Checks behaviour if the limit target is disabled:
|
""" Checks behaviour if the limit target is disabled:
|
||||||
|
Loading…
Reference in New Issue
Block a user