Conversation
74af63a to
b3a1675
Compare
|
Hi. Can you take a look at this? I would love to have some feedback :) |
|
@pmaytak This PR is a follow-up of the previous one you reviewed. We would appreciate your review here too. |
brentschmaltz
left a comment
There was a problem hiding this comment.
I wonder if there are other properties on the TimeProvider we would use besides UtcNow.
| </PropertyGroup> | ||
|
|
||
| <PropertyGroup Condition=" ($([MSBuild]::GetTargetFrameworkIdentifier('$(TargetFramework)')) == '.NETCoreApp' And $([MSBuild]::VersionGreaterThanOrEquals($(TargetFrameworkVersion), '8.0'))) "> | ||
| <DefineConstants>$(DefineConstants);SUPPORTS_TIME_PROVIDER</DefineConstants> |
There was a problem hiding this comment.
Why a new define and not just NET8+
There was a problem hiding this comment.
It is not a big reason but a matter of personal preference. It allows easy integration of Microsoft.Bcl.TimeProvider if requested.
If you prefer NET8_0_OR_GREATER, I will change it.
b3a1675 to
8fb3274
Compare
|
There is also available timezone info, GetElapsedTime, a Timer, ... I prefer to use these in a following PR. |
8fb3274 to
377bb9b
Compare
|
@iNinja and @FuPingFranco (@brentschmaltz ) are you considering this with the changes you are doing to validationParameters? |
377bb9b to
b8e5780
Compare
� Conflicts: � build/common.props
b8e5780 to
385311e
Compare
|
This pull request already laid some groundwork for enabling |
Use TimeProvider instead of DateTime.Now
This PR adds
TimeProviderfor .net8+ targets. It's done as an additional property,TimeProvider, on Descriptor/Parameter classes.Description
I added
TimeProviderto nearly all usages of DateTime.Now. Some were too difficult to add (BC Break).Fixes #2572