Report to pulls with identical SHAs #47
Conversation
gandelman-a
left a comment
gandelman-a
left a comment
There was a problem hiding this comment.
Couple of things. And I think we need a test case for this. But other than that, looks pretty straight forward..
|
|
||
| if len(pulls) > 1: | ||
| raise Exception('Multiple pulls found with head sha %s' % sha) | ||
| for pull in pulls: |
There was a problem hiding this comment.
This code wont actually run because the Exception will already have been raised at this point
| for pull in pulls: | ||
| self.commentPull(pull.owner, pull.project, pull.pr_number, | ||
| 'Multiple pulls found with head sha %s. ' | ||
| 'Please amend your change.' % sha) |
There was a problem hiding this comment.
We probably want this msg to be configurable in the same way it is for merge failures, test success, etc?
eggshell
force-pushed
the
multiple-pulls-fix
branch
from
ffb24dc to
5144626
Compare
It is possible to run into a scenario where two different pulls have the same SHA. We should report to these pulls and tell them to amend their change. Implements: BonnyCI/projman#127 Signed-off-by: Cullen Taylor <cullentaylor@outlook.com>
eggshell
force-pushed
the
multiple-pulls-fix
branch
from
5144626 to
b6b5f9a
Compare
|
Build failed.
|
|
Now we've worked with github PRs a bit more I think this situation is likely and that we need to figure out this PR->commit thing a bit bettter. This is not a -1 on this review just a conversation. It seems perfectly fine that a status resolves to multiple pull requests, we should ideally have a cache of HEAD commits on each project that we can lookup, but then just add the results of a successful status to all PRs that match. Zuul should still handle this correctly and only actually approve those that have been marked as approved. |
|
@gandelman-a I'm having a bit of difficulty with recreating the behavior in a test case. Opening two separate PRs from two different branches seems to produce no comment stream whatsoever (which is where I've left the test case in this PR). I've been testing locally with two PRs from master, but that just results in comments about successful builds. |
4 participants
It is possible to run into a scenario where two different pulls have
the same SHA. We should report to these pulls and tell them to amend
their change.
Implements: BonnyCI/projman#127
Signed-off-by: Cullen Taylor cullentaylor@outlook.com