search

tamhinsf / Azure4Alexa

Create and Host Alexa Custom Skills using .NET and Azure
Other
48 stars 21 forks source link

Fantastic contribution... but not async? #1

Closed JasonBSteele closed 8 years ago

JasonBSteele commented 8 years ago

Hi Tam,

Thank you so much for this. I've been trying to wade through the Amazon documentation trying to unpick it from AWS and Lambda but after some googling found your repo. Your excellent step by step instructions had me talking to my own custom skill in Azure within half an hour!

I did find something strange though. You opted to use SpeechletAsync but have chosen not to implement the overrides asynchronously. The signature for OnIntentAsync could look like this:

public override async Task<SpeechletResponse> OnIntentAsync(IntentRequest intentRequest, Session session)

and Tfl.Status.GetResults could also be changed to be async, so that the call to the TFL service could be done like var httpResponseMessage = await httpClient.GetAsync(tflStatusUrl); and the result read like httpResultString = await httpResponseMessage.Content.ReadAsStringAsync();.

If it would help I could submit a PR with these changes in it (it will be my first proper PR in GitHub - so you may need to work though the process together!)

I haven't even looked at the outlook and Auth 2.0 stuff yet, but that could be very useful too - thanks again!

HTH, Jason

tamhinsf commented 8 years ago

Hey there - yeah, I had those patterns originally in my code. In my struggle to learn how to get things to work, I backed out to a simpler implementation. I'll do an update soon.

Thanks! Tam

tamhinsf commented 8 years ago

Latest push should have actual async operations!