Skip to content
This repository was archived by the owner on Dec 24, 2022. It is now read-only.

Update DateTimeExtensions.cs DateTimeOffset #537

Merged
merged 1 commit into from
Feb 6, 2022
Merged

Update DateTimeExtensions.cs DateTimeOffset #537

merged 1 commit into from
Feb 6, 2022

Conversation

cthames
Copy link
Contributor

@cthames cthames commented Feb 5, 2022

Include DateTimeOffset arg to extension method ToUnixTimeMs. Made the assumption to use 'UtcDateTime' to use original DateTime implementation. Possibly a better way exists.
Note 1: Due to private static ToDateTimeSinceUnixEpoch and internal DateTimeSerializer.LocalTimeZone had to re-implement to get it to work.
Note 2: ToUnixTimeMsAlt should probably also have a DateTimeOffset implementation, but I did not implement.

Include DateTimeOffset arg to extension method ToUnixTimeMs. Made the assumption to use 'UtcDateTime' to use original DateTime implementation. Possibly a better way exists.
Note 1:  Due to `private static ToDateTimeSinceUnixEpoch` and `internal DateTimeSerializer.LocalTimeZone` had to re-implement to get it to work.
Note 2: `ToUnixTimeMsAlt` should probably also have a DateTimeOffset implementation, but I did not implement.
@cthames
Copy link
Contributor Author

cthames commented Feb 5, 2022

Or could have done something simpler and done the following. Not sure how mythz would want it. Change it to however you want.

public static long ToUnixTimeMs(this DateTimeOffset dateTimeOffset) =>
            ServiceStack.Text.DateTimeExtensions.ToUnixTimeMs(dateTimeOffset.UtcDateTime);

@mythz mythz merged commit 0c5b55c into ServiceStack:master Feb 6, 2022
@mythz
Copy link
Member

mythz commented Feb 6, 2022

Yep either works, thx for the PR 👍

@mythz
Copy link
Member

mythz commented Feb 6, 2022

FYI decided to collapse the unnecessary method call, so it's now just:

public static long ToUnixTimeMs(this DateTimeOffset dateTimeOffset) => 
    (long)ToDateTimeSinceUnixEpoch(dateTimeOffset.UtcDateTime).TotalMilliseconds;

fb5396b

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

Successfully merging this pull request may close these issues.

2 participants