slack-rs / slack-rs-api

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

allow swapping out openssl for rustls #74

Closed clux closed 5 years ago

clux commented 5 years ago

This is a pattern that is somewhat standard in other crates:

But it only actively stops you from pulling in openssl if all crates let you customise this.

dten commented 5 years ago

Thanks. Seems pretty reasonable and looks good to me from a quick glance. Will give it a quick spin in a min

dten commented 5 years ago

i take it this isn't usable from slack-rs because it uses tungestenite which doesn't have a rustls option. also the request version needs bumping to "^0.9.6" otherwise the feature doesn't exist

clux commented 5 years ago

i take it this isn't usable from slack-rs because it uses tungestenite which doesn't have a rustls option

You could make it usable from slack-rs by exporting similar features. Although it depends on what the rest of its dependencies pull in. I can see that tungstenite has some native tls option. It probably maps to the same thing, but have not tested. That's kind of an orthogonal problem because the default mode of this crate isn't changed, so it won't cause any problems for slack-rs. This is only a benefit for people using slack-rs-api directly atm.

the request version needs bumping to "^0.9.6" otherwise the feature doesn't exist

The reqwest dependency is compatible with 0.9.6 because it specifies 0.9 in both your Cargo.toml. Cargo generally picks the latest compatible with everything, but can bump the minimum to 0.9.6.

clux commented 5 years ago

Bumped it. Now if you cargo build without a lockfile it'll pick 0.9.20, but at least the minimum constraint is there.

dten commented 5 years ago

thanks