slack-rs / slack-rs-api

Rust interface for the Slack Web API
Apache License 2.0
109 stars 66 forks source link

Use async/await #88

Closed leshow closed 4 years ago

leshow commented 4 years ago

Are you interested in these changes? I think there are still a few spots in the code-gen where I'm missing the async/await annotations. I'll have to fix those.

dten commented 4 years ago

Thanks! Only had a quick skim though, will have a good check in a few days.

First off though is there any chance we can have both blocking and async options like request? Does it seems not useful?

leshow commented 4 years ago

You certainly could have both. We would have to duplicate the main trait and pass through a feature gate. From a quick glance, you'd also have to duplicate all the function call sites that just got switched to async fns. That would be a major pain.

One thing I meant to ask, what's the purpose of the string-concatenation code gen stuff that's in the tests?

seunlanlege commented 4 years ago

@leshow I'm interested in pushing this over the finish line, is there anything left to be done and if so what's left?

leshow commented 4 years ago

It depends what you want to do with it. If you don't care about maintaining sync compatibility then it currently works. If you want to provide a library that does both behind a feature gate, then you'll have to duplicate every async function and make a sync version.

I don't understand the approach taken for code generation in this library, so there may be some stuff left to do there also.

dten commented 4 years ago

sorry for letting this slide. things have been 'different' lately! I've put aside some time to try and port my existing apps to this version and see how that works out. One thing i note is the message subtype deserialising used to fall back to Message::Standard, but it wouldn't now. Would need to investigate if slack can send messages without subtype

dten commented 4 years ago

after further investigation i don't think that change should go in, also it wasn't in the generator so let's make that separate from async changes

dten commented 4 years ago

thanks @leshow

leshow commented 4 years ago

No problem!