Add new Throws overloads that allow arguments to be passed to it #1191
Conversation
There was a problem hiding this comment.
This looks great! Good work. 👍
I realise that the XML documentation comments, parameter names, etc. are a bit of a mess and not very consistent across the various files, but I think you did well in following the preexisting code patterns.
I found a few details that don't look quite right yet... but nothing major. Once those are cleaned up, this should be good to go!
|
Thanks for all the feedback, really valuable and fixed/improved things. I've left the changes as separate commits as its easier for you to see, but if you want them rebased into the others before merge then that's not a problem |
|
Also, there were quite alot of fixes needed here, so fully anticipate we might need another round of smaller fixes. |
There was a problem hiding this comment.
Thanks for making the changes, @adam-knights! I'm quite happy with the PR as it stands now, but I've found two more details that might deserve another look... if you can fix those, that would be great!
Your commits are quite nicely done already, I'll let you decide whether you prefer to merge the review commits into the earlier ones or not. Both is fine with me.
Let me know when you're ready either for me to look at the last two remaining points, or for going ahead with the merge.
|
I tidied up the commits back to a nice three and added back that test. See my thoughts on the |
|
All good, let's do this! 🚀 Thanks for your contribution. |
This implements #1048 (In
Throws, we discussed in #1190 that we would leaveThrowsAsyncalone for now).The existing method
SetReturnComputedValueBehaviorinMethodCall.csis slightly refactored so that those functions the newSetThrowComputedExceptionBehaviorneeds are available, some bits were not needed. I tried to find a balance here between duplication and the new function having its slightly different path and checks, but am more than happy to tweak this as needed. The diff here isn't great, probably better viewed side by side.