JoeRall
commented
5 years ago @rdosanjh Is there anything else you'd like to see in the PR? Would love to try and get these bits in and release an updated version of the nuget package.
Closed JoeRall closed 5 years ago
JoeRall
commented
5 years ago @rdosanjh Is there anything else you'd like to see in the PR? Would love to try and get these bits in and release an updated version of the nuget package.
ianintercom
commented
5 years ago @JoeRall - This change looks good, a test or two on this would be great! Would be happy to merge then
JoeRall
commented
5 years ago @ianintercom - Thanks for the feedback. I just posted some tests. Note that I didn't see any examples of using MOQ in this repo so I took a stab at one possible way to test the clients. I think a safer set of tests would require a little refactoring so we can override some of the protected methods inside the Client base class. But I didn't want the scope of this change to get too big. I think that'd warrant it's own pull request.
Let me know what you think and I'll make any necessary modifications as necessary. I'd really love to get these changes out the door and on nuget.
ianintercom
commented
5 years ago Hey @JoeRall, nice work on this! As of a couple of days ago, we are looking at a refactor at the moment which should greatly improve the test-ability of the clients. Once we have this merged, I think merging in your original change and apply the new test pattern shouldn't be a problem. Sorry for the back and forth on this, I hadn't looked into the testing in this repo properly before asking you for test, my bad!
JoeRall
commented
5 years ago @ianintercom No worries at all. That makes sense if there's a big refactoring that's about to go down. I'll keep an eye on #145
Let me know if there's anything else I can help with.
Why? Updated model that's returned when replying to a message.
See Docs
How? Simply changed the return value to be a a Conversation vs a ConversationPart as the documentation expects.