Obsolete ILocalClock.LocalNowAsync #16752
Conversation
src/OrchardCore/OrchardCore.Abstractions/Modules/Services/ILocalClock.cs
Outdated
Show resolved
Hide resolved
src/OrchardCore/OrchardCore.Abstractions/Modules/Services/ILocalClock.cs
Outdated
Show resolved
Hide resolved
| /// <summary> | ||
| /// Gets the time for the local time zone. | ||
| /// </summary> | ||
| Task<DateTimeOffset> LocalNowAsync { get; } | ||
|
|
||
| Task<DateTimeOffset> GetLocalNowAsync(); |
There was a problem hiding this comment.
This introduces a breaking change. To avoid having a breaking change so this instead
Task<DateTimeOffset> LocalNowAsync { get; }
Task<DateTimeOffset> GetLocalNowAsync()
=> LocalNowAsync ;
There was a problem hiding this comment.
Why it's a breaking change? the value still comes from the new implementation, right?
There was a problem hiding this comment.
It's for when you have an existing alternative implementation. Should be rare but can happen: Then, with Mike's code, no change is needed, only when we remove LocalNowAsync.
There was a problem hiding this comment.
That's why I used error before to let the compiler complains whenever we use it
There was a problem hiding this comment.
That's why I used error before to let the compiler complains whenever we use it
this will break people. So we have to avoid it. Doing my suggestion then no breaking change because in the interface we provide the default implementation. In your implementation, you can change the default behavior found in the interface.
|
|
…alClock.cs Co-authored-by: Zoltán Lehóczky <[email protected]>
Fixes #16751
I'm not sure why we added an
asyncproperty which was the first time I saw it in my life. @rjpowers10 could you please try to reproduce in this PR