Closed daniestevez closed 8 months ago
~Hold on merging. I wasn't testing with --all-features
. I'll fix the problems that appear and force-push.~
False alarm. The features are mutually exclusive. I've now tested
cargo test && cargo test --features yaml_conf --no-default-features && cargo test --features ron_conf --no-default-features
and it passes.
Lets see what CI tells us đź‘Ť
All version change are "major" indicating a breaking change for each. Are they in our public API or not?
I mean do their change also impact us? (For example: if the directories
crate changed a config folder in any OS it is breaking for us too.)
I don’t mean it to block the PR, just to have an idea of the next confy version.
Fair enough. Taking a look at the changes.
directories 4 -> 5 updates dir-sys from 0.3.6 to 0.4.0 (plus an added function for Windows which doesn't seem to impact confy). dir-sys doesn't seem to have a CHANGELOG, but looking at the commit log, there doesn't seem to be anything impacting confy (the only major change is moving from winapi to windows-sys).
serde_yaml 0.8 -> 0.9. Here are the 0.9 release notes. Looking at the breaking changes, there are a couple of changes in the serialization format. I guess that for confy this means that if the configuration file does not exist and a default gets created, it can be formatted differently now. I don't know if this would be considered an API-breaking change. Another change is "A bunch of non-base-10 edge cases in number parsing have been resolved. For example 0x+1 and ++0x1 are now parsed as strings, whereas they used to be incorrectly treated as numbers.". If someone was misusing this in their configuration, this will probably break things for them.
toml 0.5 -> 0.8. Here is the CHANGELOG. The only breaking change that got my attention is "Serialization and deserialization of tuple variants has changed from being an array to being a table with the key being the variant name and the value being the array". It seems that this changes how default confy configurations are formatted, and it might also prevent older configurations from being parsed. The other changes are API changes that don't impact confy and are not in the confy public API.
Wow thanks a lot for the detailed breakdown ! :+1:
I think it means (at least because of serde_yaml and toml) that we should do a breaking version too.
sorry this is late!
Thanks for doing this, nice that no code changes were needed!
Before merging I would like to make sure we have some backwards compatibility between default configs, I'll look into this and get this merged and a new version (major) out.
Just an update, I'll find time this week -> weekend to test this. Sorry again for the delay
Yup I think this is common enough we will definitely need to do a breaking version number change just based on the toml
bump alone.
The following config will produce 2 different default-config.toml
files that do not (de)serialize nicely:
#[derive(Debug, Serialize, Deserialize)]
enum Tup {
O(u8, u8, u8),
}
#[derive(Debug, Serialize, Deserialize)]
struct MyConfig {
One: u8,
Two: String,
Three: Tup,
}
current stable
One = 0
Two = ''
Three = [
1,
2,
3,
]
This branch:
One = 0
Two = ""
[Three]
O = [
1,
2,
3,
]
The old config will fail this branch with:
warning: `toml_tuple` (bin "toml_tuple") generated 3 warnings
Finished dev [unoptimized + debuginfo] target(s) in 0.01s
Running `target/debug/toml_tuple`
Error: BadTomlData(Error { inner: Error { inner: TomlError { message: "wanted string or table", original: Some("One = 0\nTwo = ''\nThree = [\n 1,\n 2,\n 3,\n]\n"), keys: ["Three"], span: Some(25..49) } } })
That being said, let's go ahead and get this merged. I am going to use your blurb for the serde_yaml
and toml
breaking changes in the README
and publish a 0.6
release.
Thanks for looking into this!
This updates the dependencies to their latest version. No API changes of these updates impact confy, so no code changes are needed. The code builds and the tests run.
This closes #87.
I have not updated the crate version in
Cargo.toml
, but I can do that as part of this PR so that a new version can be published.