snafu + thiserror: why both?

[olix0r]

kube-runtime has a dependency on snafu; Kube uses this thiserror. Generally, it seems preferable for libraries to avoid 'framework' error types like those provided by snafu. Is there any interest in a PR that removes the dependency on snafu?

I only noticed this because cargo-deny flags that snafu pulls in older versions of pin-project. (It's a bit surprising to me that an error framework even needs to worry about pinning at all):

kube-runtime v0.58.1 (/home/ver/b/kube-rs/kube-runtime)
...
│ ├── kube v0.58.1 (/home/ver/b/kube-rs/kube)
...
│   ├── kube-core v0.58.1 (/home/ver/b/kube-rs/kube-core)
...
│   │   └── thiserror v1.0.24
...
│   ├── pin-project v1.0.7
│   │   └── pin-project-internal v1.0.7 (proc-macro)
...
│   ├── thiserror v1.0.24 (*)
...
├── pin-project v1.0.7 (*)
...
├── snafu v0.6.10
│   ├── doc-comment v0.3.3
│   ├── futures-core v0.3.15
│   ├── pin-project v0.4.28
│   │   └── pin-project-internal v0.4.28 (proc-macro)
...

[nightkr]

SNAFU is a transparent error-derive crate, just like thiserror (the title suggests that you might be confusing it with anyhow, which AFAIK we shouldn't have any dependencies on?).

As for why we use both.. that is mostly a historical accident. There was some discussion in #453 that ended up in a loose consensus to migrate towards SNAFU, but nobody has taken the reins on actually doing that yet. PRs would be welcome there.

I only noticed this because cargo-deny flags that snafu pulls in older versions of pin-project. (It's a bit surprising to me that an error framework even needs to worry about pinning at all):

This is for https://docs.rs/snafu/0.6.10/snafu/futures/trait.TryFutureExt.html and https://docs.rs/snafu/0.6.10/snafu/futures/trait.TryStreamExt.html, which add error handling helpers to Futures and Streams.

SNAFU 0.7.0 (currently in beta) seems to have upgraded to pin-project 1.0, fwiw: shepmaster/snafu#255

[olix0r]

Sorry, the title should have referred to snafu. It does look like anyhow is also used but only as a dev-dependency:

tests v0.1.0 (/home/ver/b/kube-rs/tests)
├── anyhow v1.0.40

But this shouldn't really a problem for consumers.

I missed the discussion in #453 -- thanks for pointing this out. I guess my larger point is that thiserror is already in the dependency graph, so it feels a bit unnecessary to pull in another system for this; but I think the other discussion gets deeper into the tradeoffs.

Thanks!