-
-
Notifications
You must be signed in to change notification settings - Fork 75
Conversation
Thanks for the pull request, @flying-sheep! I took a look to make sure it's ready for merging and found some changes are needed:
Can you please update the pull request to address these? (More information can be found in our pull request guide.) |
LGTM |
travis failure is due to #128 please fix travis or ensure manually that my tests run with TypeScript 2.0.10. I beg you to merge this quickly so that I don’t have to adapt this to every unit test change merged before this. |
I'm sure you can appreciate that the week before Christmas is exceptionally busy. Rest assured, I will be looking at this as soon as possible. |
oh, of course! i don’t want to pressure you, is just wanted to ask you to consider fast-tracking this to get in before other PRs or changes |
Thanks a lot for your patience on this, @flying-sheep! Once #131 gets merged you can rebase this |
It's in! Please rebase the PR and we'll see where we're at 😄 |
LGTM |
@JamesHenry done! |
Thanks a lot, glad to see that addressed the failing tests! Given that they both touch a lot of existing assertions around functions, I think it might be sensible to hold off on this PR just until this other one gets resolved: #124 Shouldn't take too much longer! |
The other PR ended up being more isolated than it first appeared. This LGTM, thanks a lot! |
great, thanks! sorry again for sounding pushy, y’all’re doing a great job here! |
I split this into two commits, because the second one is a rather mechanical addition of
async: false
to every single fixture containing a function (=almost all)(fixes #119)