Update TimeOnly ToXmlString to include time zone offset#44
Update TimeOnly ToXmlString to include time zone offset#44GregoryNikolaishvili merged 3 commits intomainfrom
Conversation
- Change ToXmlString(TimeOnly) to output "HH:mm:sszzz" with local offset - Update unit test to match new format with time zone info - Bump Verify.Xunit package to v31.12.5
GregoryNikolaishvili
assigned
GregoryNikolaishvili and
temonk and unassigned
GregoryNikolaishvili
There was a problem hiding this comment.
Pull request overview
Updates TimeOnly.ToXmlString() output to include a time zone offset, aligns the unit test with the new format, and bumps the Verify.Xunit dependency.
Changes:
- Change
ToXmlString(TimeOnly)to format asHH:mm:sszzzusing aDateTimeOffset. - Update the corresponding unit test to accept the new offset-bearing format.
- Bump
Verify.Xunitpackage version to31.12.5.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/AltaSoft.DomainPrimitives/ToXmlStringExt.cs |
Updates TimeOnly XML serialization to include a time zone offset. |
tests/AltaSoft.DomainPrimitives.XmlDataTypes.Tests/ToXmlStringExtTests.cs |
Adjusts the TimeOnly formatting assertion for offset output. |
Directory.Packages.props |
Updates Verify.Xunit version used by tests. |
| /// <summary> | ||
| /// Converts a <see cref="TimeOnly" /> value to its XML string representation in the format "HH:mm:ss". | ||
| /// </summary> |
There was a problem hiding this comment.
The XML doc comment still states the TimeOnly XML format is "HH:mm:ss", but the implementation now returns "HH:mm:sszzz" (includes a time zone offset). Update the summary (and any referenced format text) so the documentation matches the new behavior.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
There was a problem hiding this comment.
Done in 79a6b3a. Updated the XML doc comment to say "HH:mm:sszzz" and also strengthened the test to verify the offset suffix matches the actual current local offset (not just the format shape).
| public void TimeOnly_ToXmlString_ReturnsExpectedFormat() | ||
| { | ||
| var t = new TimeOnly(13, 45, 30); | ||
| var xml = t.ToXmlString(); | ||
| Assert.Equal("13:45:30", xml); | ||
| Assert.Matches(@"^13:45:30(?:Z|[+-]\d{2}:\d{2})$", xml); | ||
| } |
There was a problem hiding this comment.
The test only validates the output shape via regex; it doesn’t confirm the offset is actually the local offset. If the intended behavior is specifically “local offset”, consider asserting the suffix equals the current local offset string (e.g., derived from DateTimeOffset.Now) once the production code derives the offset from “now” rather than from an arbitrary local date/time.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Agent-Logs-Url: https://github.com/altasoft/DomainPrimitives/sessions/5b0d89b2-92b7-4b7f-8c6c-75dfa3befa56 Co-authored-by: GregoryNikolaishvili <25057879+GregoryNikolaishvili@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.