-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Conversation
Thank you for your contribution. It's not clear to me what issue is addressed by this change. As some time has passed since this was submitted and there has been no follow-up for a while, I think it's safe to close. Will happily reopen if this is something you're still looking to get merged in, (but I will need to understand it first.) |
@kingdonb this fixes a big in the mock. The mock is called a function on itself instead of mock handler function. If this function was called it would just get in an infinite recursive loop. Hope this explains the issue. |
Thanks for your reply. So, did nobody notice because this mock was essentially unused? I appreciate your taking a moment to explain the issue for my benefit. If the mock is unused, it seems like it should simply be removed, (but as I've only just begun to understand the issue myself, I will reserve judgment until I can take some more time to understand it.) Safe to say then, it's not putting you out at all if it doesn't get included in the next release which is planned for later this week? |
(The DCO bot will not allow me to merge this unless you rebase/squash and add a DCO sign-off with |
No there is no rush with this. I came across this by accident. The methods on the mock are definitely not used otherwise tests would end in an infinite recursive call. I had a recollection that other functions on the mock are being used and that it implemented a cluster interface. My memory is a little hazy on this though. |
@kingdonb rebased to master and signed off the commit. Hope everything looks okay. |
Signed-off-by: Ben Wells <b.v.wells@gmail.com> Signed-off-by: Kingdon Barrett <kingdon@weave.works>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Thanks for the review and merge @kingdonb! |
Fix recursive calls in mock