From 3f61dc9ce4a0e502c1a9e39877deee90567a30dc Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 30 Jan 2020 13:56:01 +0100 Subject: [PATCH] [IMP] forwardport: warning message when co-dep PR has a conflict * Add some more information as to why the user *should* do on the PR the message is printed on, the previous message left that to their imagination * The PR selection was *completely* wrong as it would select the old PRs which really isn't what we want. And turns out there's no good reason to create & send the feedback in the loop creating the forward-port prs, that can be moved to a followup loop where we have created hopefully created all the forward-port PRs. Also technically we could do even better than currently and remap the prs mapped to conflict data to the new PRs and know exactly which of the forward-ported PRs is faulty, but that seems overkill for now. --- forwardport/models/project.py | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/forwardport/models/project.py b/forwardport/models/project.py index 88e7edd4..774917e7 100644 --- a/forwardport/models/project.py +++ b/forwardport/models/project.py @@ -602,8 +602,6 @@ class PullRequests(models.Model): for p in root | source ) - (h, out, err) = conflicts.get(pr) or (None, None, None) - title, body = re.match(r'(?P[^\n]+)\n*(?P<body>.*)', message, flags=re.DOTALL).groups() title = '[FW]' + title if not body: @@ -655,13 +653,24 @@ class PullRequests(models.Model): (4, new_pr.id, False), ] }) + # 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) = conflicts.get(pr) or (None, None, None) footer = '\nMore info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port\n' if has_conflicts and not h: - footer = '\nWarning: at least one co-dependent PR (%s) did ' \ - 'not properly forward-port, you will need to fix it ' \ - 'before this can be merged\n%s' % ( - ', '.join(p.display_name for p in conflicts), + 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 ) @@ -710,10 +719,6 @@ This PR targets %s and is part of the forward-port chain. Further PRs will be cr 'to_add': json.dumps(labels), 'token_field': 'fp_github_token', }) - # 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() # batch the PRs so _validate can perform the followup FP properly # (with the entire batch). If there are conflict then create a