Protocol tests issue4 #3243
Protocol tests issue4 #3243
Conversation
boblodgett
changed the base branch from
protocol-tests
to
protocol-tests-1.0.96.0
| /// Converts a DateTime to ISO8601 formatted string with milliseconds | ||
| /// if they are not zero. | ||
| /// </summary> | ||
| public static string FromDateTimeToISO8601Minimal(DateTime value) |
There was a problem hiding this comment.
What does word "Minimal" have to do with including milliseconds? Shouldn't this be FromDateTimeToISO8601WithMs?
There was a problem hiding this comment.
It will use minimal ms length. Meaning if there are ms != 000 it will include it else it won't. I am open to other names but the above suggestion isn't completely correct as it may not have ms. We also already have a with and without ms method but not a smart one that includes ms if not 0.
There was a problem hiding this comment.
Yeah that is hard to name. Maybe FromDateTimeToISO8601WithOptionalMs? Either way it is going to come down to the docs which you do have. I'll approve the PR.
There was a problem hiding this comment.
Updated to the new suggested method name FromDateTimeToISO8601WithOptionalMs as it does make it clearer and is accurate.
|
Not related, but I think we can check the "I have added test to cover my changes" for each issue, since we do have protocol tests related to each issue we are fixing. |
Description
This PR fixes DOTNET-7357 issue 4.
Issue 4 Rest-Json Protocol : Incorrectly marshalling dateTimes in the URL.MERGE into
protocol-testsfeature branch onlyBuilds on prior PR: #3242
Merge order: 2
Ticket: DOTNET-7357
Motivation and Context
Testing
N/A - the test cases are the protocol tests. A final dry-run will be done on the
protocol-testsfeature branch once everything is merged into the feature branch.Screenshots (if appropriate)
Types of changes
Checklist
License