From 4d528465d9083b1bf4346c6d0422d2d919f1140e Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Tue, 19 May 2020 14:13:21 +0200 Subject: [PATCH] [FIX] forwardport: incorrect / misleading conflict message Given a PR batch getting forward-ported together, if one of the PRs has a conflict the others should be considered "in conflict" as well, and should have a note pointing in that direction and indicating that the PR should be approved the normal way eventually. Which they do. However, the message is confusing as it gets bolted on the normal non-conflicting message, either noting that it's part of a chain or (worse because it gives conflicting indication) the "terminal" message recommending using the forwardbot to approve of the entire chain. I've no idea why I did it that way instead of just adding a case to the conditional, and the commit message provides no indication. But perform that change, it seems innocuous, hopefully there weren't good reasons I forgot about for doing it the other way around. Fixes #367 --- forwardport/models/project.py | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index a9421f91..417787c8 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -36,7 +36,7 @@ from odoo.tools.appdirs import user_cache_dir from odoo.addons.runbot_merge import utils from odoo.addons.runbot_merge.models.pull_requests import RPLUS -FOOTER = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n' +footer = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n' DEFAULT_DELTA = dateutil.relativedelta.relativedelta(days=3) @@ -656,7 +656,7 @@ class PullRequests(models.Model): message = source._pingline() + """ 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) +%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, @@ -672,18 +672,6 @@ The next pull request (%s) is in conflict. You can merge the chain up to here by source = pr.source_id or pr (h, out, err) = conflicts.get(pr) or (None, None, None) - footer = FOOTER - if has_conflicts and not h: - footer = '\n**WARNING** at least one co-dependent PR (%s) ' \ - 'did not properly forward-port, 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".\n%s' % ( - ', '.join(p.display_name for p in (new_batch - new_pr)), - proj.github_prefix, - footer - ) - if h: sout = serr = '' if out.strip(): @@ -698,6 +686,17 @@ Either perform the forward-port manually (and push to this branch, proceeding as In the former case, you may want to edit this PR message as well. """ % (h, source.display_name, sout, serr) + 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._pingline(), + ', '.join(p.display_name for p in (new_batch - new_pr)), + proj.github_prefix, + footer + ) elif base._find_next_target(new_pr) is None: ancestors = "".join( "* %s\n" % p.display_name