Added Async support #9
Conversation
…to avoid breaking changes by default. Added more tests for UNWRAP_EX flag.
|
What is |
|
My bad. I replaced SYNC_ONLY flag with ASYNC and added UNWRAP_EX flag to avoid breaking changes. In addition I tweaked the test project so that can be tested for each flag or combination of them. |
|
@JakeGinnivan Would you mind giving this PR a once over? There are a lot of changes. From initial glance, it looks pretty good. But I haven't written C# in a while and am not up to date on the various platform changes... |
|
@staxmanade I'll take a look as well latter today, as we use it in @Resgrid. |
| public class AsyncSample | ||
| { | ||
| public static void Run() | ||
| { | ||
| var config = new EventAggregator.Config | ||
| { | ||
| // Make the marshaler run in the background thread | ||
| DefaultThreadMarshaler = action => Task.Factory.StartNew(action), | ||
| DefaultThreadAsyncMarshaler = async action => await Task.Factory.StartNew(action), |
There was a problem hiding this comment.
You don't need the async/await pair. Task.Factory.StartNew will never throw (and the only difference async/await will make is that Run() will end up in the call stack of an exception)
There was a problem hiding this comment.
I think this should be renamed back and the marshal param removed from the async send message overloads
There was a problem hiding this comment.
Removed the async/await pair. The DefaultThreadMarshaler is still there I just added the async one as Action<> can not be awaited.
|
@staxmanade I haven't used this project for a while, so some of the comments might be off (is this a content nuget package or a distributed binary for instance?). @maca88 the main thing is to add |
|
@JakeGinnivan Nice comments on the PR. .ConfigureAwait(false) is a biggie. Nuget distribution is content (drops a .cs file into an Events folder). Really good catch on the Wait() as well. @maca88 Is there anyway to stand up the Async methods alongside the current ones? Preprocessor directives are sometimes necessary, but tend to complicate the API surface when being used to hide implementations. Really depends on where the breaking changes are. |
…5.2, added stack trace preservation for .net profile client, added configure await for all awaits, replaced wait with configure await.
|
@ucswift I wrapped all async stuff into ASYNC directive in order to support .NET 4 as well. Also the UNWRAP_EX was added in order to not having any breaking changes. If you don't care about supporting .NET 4 I can remove the ASYNC directive. |
|
@maca88 That makes more sense, using the directives to support .Net 4. I don't know where the value lies in support 4.0 verses bumping support to say 4.5, guess that would be an @staxmanade decision. |
|
Hey All Unfortunately I haven't used this library since StatLight 😢 . I also have been a bit out of touch with all of the various So if I can get a 👍 from at least @ucswift or @JakeGinnivan on this PR - then I'll merge it and re-figure out how to publish it to NuGet... @maca88 - thanks for you're hard work... |
Async can be disabled by adding SYNC_ONLY flag.