Fixed build warnings in src/Nancy #2667
Conversation
|
👀 @NancyFx/most-valued-minions |
| @@ -188,7 +188,7 @@ private static Response ExecuteDiagnostics(NancyContext ctx, IRouteResolver rout | |||
|
|
|||
| if (resolveResult.After != null) | |||
| { | |||
| resolveResult.After.Invoke(ctx, CancellationToken); | |||
| resolveResult.After.Invoke(ctx, CancellationToken).Wait(); | |||
There was a problem hiding this comment.
Yeah I know and I had a chat with @damianh about it.. the real alternative would be to convert the diagnostics stuff completely to async as well I guess?
There was a problem hiding this comment.
Yeah, we must not forget that we have a pretty half-assed async story right now. I think we really need to look at getting everything async soon -- before we release 2.0.
There was a problem hiding this comment.
Yeah just gave a nudge at #2577
There was a problem hiding this comment.
I can revert the Wait() so we can get the other stuff in?
There was a problem hiding this comment.
Yeah, let's tackle everything when we're ready for it 😝
There was a problem hiding this comment.
This is one of those changes that will have a knock on effect and will explode this PR. I would agree to merge this PR as is and fix this .Wait() in a subsequent PR.
thecodejunkie
force-pushed
the
fix-build-warnings
branch
from
f795b04
to
43f198b
Compare
|
We're actually going a |
|
Yeah, just nuke it while you're there 😄 |
|
Argh. The damn |
thecodejunkie
force-pushed
the
fix-build-warnings
branch
from
43f198b
to
324de19
Compare
|
@khellang removed the second |
Prerequisites
Description
Fixed build warnings. The following was changed
this.Stringtostringif-statement in curly-bracesThe one code change that went in was in DiagnosticsHook where we don't really do async handling and we were calling on a method that returns
Task. I've changed that toWait()on the task