Support for humanizing DateTimeOffset #399
Conversation
|
Hi @tompazourek. Awesome PR and we really needed this. I am very keen to merge this in; but there are a couple of things I would do differently which I've commented on. Please address these, and leave me a comment and I will merge it. P.S. I changed the description to say 'Fixes' instead of 'issue'. That will auto close the issue when PR's merged. |
| @@ -273,12 +273,13 @@ DateTime.UtcNow.AddHours(30).Humanize() => "tomorrow" | |||
| DateTime.UtcNow.AddHours(2).Humanize() => "2 hours from now" | |||
There was a problem hiding this comment.
Please add an example for DateTimeOffset here.
|
This is one clean first PR you've submitted. Awesome work. If you could just please fix the issues I pointed that would be awesome. Please also add the PR to the releasenotes file. Cheers |
tompazourek
changed the title
|
Thanks for the feedback. I separated the implementation of strategies for DateTime and DateTimeOffset, as you suggested. First I experimented with a base class for each pair of strategies, but I felt that the solution was too rigid and the class names were just too long and not nice. So I decided to move the two algorithms to separate |
|
Cool. Thanks. |
Support for humanizing DateTimeOffset
|
This is now released to NuGet as v1.35.0. Thanks for the contribution. |
I added support for humanizing DateTimeOffset (fixes #379).
It's one of my first pull requests ever, so please comment if I did not do something correctly.
To preserve the backwards compatibility, I did not change the
IDateTimeHumanizeStrategyinterface, but added newIDateTimeOffsetHumanizeStrategyinterface.The implementation of these strategies included in the library (
DefaultDateTimeHumanizeStrategyandPrecisionDateTimeHumanizeStrategy) now implement both interfaces, since they share the same algorithm and 99% of the implementation. I thought this was the best solution so as not to introduce compatibility issues.Support for the
DateTimeOffsetchanges the public API (PublicApiApprovalTest.approve_public_api.approved.txt) a little bit. I am not sure what is the approval process for the API here, but I updated the API there.Any feedback and ideas are welcome.