aatxe / irc

the irc crate – usable, async IRC for Rust
Mozilla Public License 2.0
528 stars 97 forks source link

Migrated to `tokio` handleless runtime with `impl Future` result types. #141

Closed Ratysz closed 6 years ago

Ratysz commented 6 years ago

Key features:

More detailed breakdown is in commit messages.

To be perfectly honest, I'm not sure there's no way to not take ownership of Config, but I couldn't find one that I'd be comfortable implementing.

EDIT: Forgot to add reasoning, if for posterity's sake; this change permits users to roll own tokio runtimes and spawn IrcClient's futures into them.

aatxe commented 6 years ago

Hi @Ratysz!

Thanks for a great PR! I'm fine with the copying of Config for now (though as I mentioned on Discord, I think it'd be nice if Config was copy-on-write since I think mutation of it is rare). I'm very excited to see these simplifications for all of our async code! I hope people might even be more inclined to just use tokio directly. 😉

It seems CI is failing because you didn't update the doc test in the README though. I'll add something about checking that the examples there still work to CONTRIBUTING.md, but would appreciate if you can fix it before merging.

Cheers! 🍻

Ratysz commented 6 years ago

Whoops, I was afraid something might've fallen through the automated suite. Should be fixed now (even if I did commit to the wrong branch on my fork; twice)!

Yes, I remembered about the Cow, and I tried to make use of it - doesn't click yet. I'm sure if I come back to it in a week or two I'll be able to figure something out.

aatxe commented 6 years ago

Cow is but one way to do copy-on-write. I'm not sure if it would be annoying from a programming perspective (it certainly might be), but we could have a similar effect by replacing today's Config with Arc<Config> (and using Arc::make_mut when mutation is necessary). But this is indeed a problem for another day and another PR. Today, we merge this one! 😄