Conversation
…arse key containing periods in yaml
|
Nice. Could you also add one with I will then take these patches and continue based on them... not sure how, but I guess I will think of something. |
|
@matthiasbeyer not sure I understand; the default case is |
|
Yes, I meant add another case where you use only |
|
@matthiasbeyer Done. Surprising result, the new test passes |
matthiasbeyer
left a comment
There was a problem hiding this comment.
The changes itself look good.
CI fails because the files in tests/Settings are not updated to match the expected Settings structure!
The commit linter also fails. Please:
- Rewrite the commits in imperative mood ("Add stuff" instead of "Adding stuff"). Don't hesitate to include details in the message body!
- Ensure the length of the subject line does not exceed 50 characters
- Make sure to sign off your commits
In general: https://cbea.ms/git-commit/ 😉
Or ping me if you don't care anymore, then I will take over this PR! 😉
| } | ||
|
|
||
| #[test] | ||
| fn test_file() { |
There was a problem hiding this comment.
This test fails because the files in tests/Settings are not updated for the new key ("192.168.1.1")!
| fn test_keys_with_periods_deserialize() { | ||
| let c = make(); | ||
|
|
||
| let s: Settings = c.try_deserialize().unwrap(); |
There was a problem hiding this comment.
This test fails here because the tests/Settings files are not updated.
There was a problem hiding this comment.
@matthiasbeyer It was updated, but due to #417 it fails. This is the bug reproduced.
There was a problem hiding this comment.
Ah, I guess I was not fully awake when reviewing this. Sorry about that.
|
So if I understand correctly:
Okay, so we have to investigate why that is. |
|
This is not specific to YAML parsing, but feature of
Failure is due to Instead That above code is called from this method during config build: which multiple locations will call via this method: I'm not too familiar with I understand the intent was to deserialize with the serde rename feature to the desired field. This won't work AFAIK since the builder is creating this # Example input
hello.world: example
192.168.1.1: a string valueBuilt config before deserializing to target config from userConfig format deserializes correctly before handed to builder as source input |
No description provided.