-
Notifications
You must be signed in to change notification settings - Fork 60
Conversation
Upgrade Newtonsoft.Json reference to v. 9.0.1 (lowest version with netstandard support) Add reference to System.Threading.Thread
…not available in netstandard
…ay to do it in netstandard
…quest, as that is not supported on netstandard Change the MakeRequest method to be async because HttpClient is async
…wait for the async requests
We cannot go lower than 1.3 as we need support for multithreading
Now changed the API back to synchronous to avoid breaking changes |
@@ -71,7 +65,7 @@ internal class AsyncFlushHandler : IFlushHandler | |||
_flushingThread.Start(); | |||
} | |||
|
|||
public void Process(BaseAction action) | |||
public async Task Process(BaseAction action) |
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.
Should this be left void
?
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.
It depends on which layer in the application you want to switch from async to sync. In my pull request I suggest to do that switch in the topmost layer (the client). That makes is easy to add async overloads of the client methods if you want that in the future (which I think would be a good idea).
However, we can also choose to do that in the lower layer, e.g. where we are making the http requests. That means we can keep the rest of the layers without modification.
I don't see the harm in changing this method to async, as it is an internal class - so I don't consider this a breaking change.
In any case, the decision is yours. Let me know if you need me to modify it.
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.
I forgot this was internal. This looks good!
Hi guys
Here is a pull request for changing the library to target netstandard 1.3.
As a part of doing this update I made a breaking change: All the public methods on the Client are now async. I realise you might not want this, so let me know if I should change them back to be synchronous again.
Thanks!