Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replaced DateTimeService with System.TimeProvider #884

Closed
wants to merge 1 commit into from

Conversation

tonven
Copy link

@tonven tonven commented Jun 29, 2023

Starting from .NET 8 there is an inbuild TimeProvider Class which can be used as an abstractrion over DateTime. https://learn.microsoft.com/en-us/dotnet/api/system.timeprovider?view=net-8.0

@neozhu
Copy link

neozhu commented Jun 29, 2023

I think it's better to save in UTC time, and convert to local time for display when outputting.

@tonven
Copy link
Author

tonven commented Jun 29, 2023

I think it's better to save in UTC time, and convert to local time for display when outputting.

I agree. I was also thinking about changing Created and LastModified to DateTimeOffset and saving it to UTC time.

@jasontaylordev what do you think?

@jasontaylordev
Copy link
Owner

I think it's better to save in UTC time, and convert to local time for display when outputting.

I agree. I was also thinking about changing Created and LastModified to DateTimeOffset and saving it to UTC time.

@jasontaylordev what do you think?

I agree, thanks for the contribution. 🙂

@jasontaylordev
Copy link
Owner

Thanks again @tonven. Once the final changes come through, I'll review and merge.

@rlarno
Copy link

rlarno commented Jul 5, 2023

If you change to using UTC, I suggest to also reflect that to the property names CreatedUTC and LastModifiedUTC

What these values need to contain so depend on the usecase. If these properties are for 'operational' reasons, UTC is the best value, as you can correlate typically with server logs which also (should) have UTC. If these values are to be used to define when the user created/modified the TodoItem, a full DateTimeOffset should be used, unless you are sure to always be operating in a single timezone.

@jasontaylordev
Copy link
Owner

Closing due to inactivity, feel free to raise again.

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.

4 participants