search

rust-db / refinery

Powerful SQL migration toolkit for Rust.
MIT License
1.35k stars 126 forks source link

serde properties of Config should be under a feature gate along with Toml serde impl #308

Closed vasilakisfil closed 9 months ago

vasilakisfil commented 10 months ago

Apparently Config already allows you to config it programmatically. However, refinery-core (used by refinery) uses serde by default to support serialization and deserialization of this type.

This is unnecessary if you don't use refinery-cli and configure it programmatically (using the set_xxx methods).

On top of that, refinery makes the assumption that Toml will be used to deserialize/serialize this type, as toml dependency is used as well in refinery-core by default. Again it's not always the case, this is more related with refinery-cli.

I suggest declaring both toml and serde dependencies as optional, under a feature flag. They can appear under the default-features, if they are used so often. It will also help not breaking anyone's code.

jxs commented 10 months ago

Hi, yeah makes sense, would you like to submit a PR addressing this? Thanks!

vasilakisfil commented 10 months ago

@jxs take a look at #310