Skip to content

Conversation

@nzbart
Copy link

@nzbart nzbart commented Oct 23, 2022

The ToString method on CalDateTime has useful logic to render the date and/or time components, but this rendering ignores the format provider and always uses the current culture. The format provider is currently used when the format string is specified, but not otherwise.

This PR adds some characterisation tests and alters ToString to use the format provider for date formatting.

I want to make a change to CalDateTime.ToString and want to be confident
that I'm not breaking existing functionality. These tests pass against
the current code.
When using the default format string, callers still want to be able to
specify a formatter to make sure that dates are formatted in their
preferred way.
CalDateTime
* to use DateOnly and TimeOnly types
* Added SetValue(DateOnly date, TimeOnly? time, DateTimeKind kind) to set the value of the date and time properly.
* Refactored `ToString` overloads to convert the date and time to a string representation.
* marked some properties as obsolete.
* Updated related tests and methods to accommodate these changes.
* Increase code coverage > 90%

DateTimeSerializer
* Added validation in the `SerializeToString` method to ensure the TZID parameter is correctly handled based on whether the date-time is in UTC.
* Improved the Deserialize method to handle both full date-time and date-only strings, initializing the DateOnly and TimeOnly parts accordingly.
* Increase code coverage > 90%

Added new package reference `Portable.System.DateTimeOnly` for netstandard2.0 and netstandard2.1 for compatibility with net6.0 and later.

Fixes ical-org#630
@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 4, 2024

@axunonb
Copy link
Collaborator

axunonb commented Nov 4, 2024

The PR is included in #631. Thanks for posting!
Sorry for the confusion when pushing mistakenly to your PR.

@axunonb axunonb closed this Nov 4, 2024
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.

2 participants