Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(datetime): impl Serialize/Deserialize for Date/Time #771

Closed
wants to merge 0 commits into from

Conversation

loqusion
Copy link
Contributor

Closes #763

Comment on lines 1374 to 1377
let err = toml::from_str::<Document>("time = 2024-01-01T05:00:00").unwrap_err();
assert_data_eq!(
err.message(),
str!["expected only time portion of datetime"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All commits should be passing. When adding tests in a commit before doing feature work, make them show the current behavior.

See https://github.com/toml-rs/toml/blob/main/CONTRIBUTING.md#process

Add tests in a commit before their feature or fix, showing the current behavior. The diff for the feature/fix commit will then show how the behavior changed, making it clearer to reviewrs and the community and showing people that the test is verifying the expected state.

e.g. clap-rs/clap#5520

time: None,
offset: None,
} => Ok(date),
_ => Err(de::Error::custom("expected only date portion of datetime")),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like Error::invalid_type would be more appropriate for these

@epage
Copy link
Member

epage commented Jul 30, 2024

Weird, unsure why thnis was closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Serialize and Deserialize for Date and Time
2 participants