Make BclDateTimeZone behave more reasonably for midnight transitions #1527
Conversation
| var expectedFallBack = Instant.FromUtc(2012, 2, 26, 2, 0, 0); | ||
| var expectedSpringForward = Instant.FromUtc(2012, 10, 21, 3, 0, 0); | ||
|
|
||
| // We assume that the fall-back happens before the spring-forward in the dates |
There was a problem hiding this comment.
Unfinished comment? (Also, do we have to assume anything? This test is for a concrete situation.)
| Assert.AreEqual(expectedSpringForward, nodaTzdbInterval.End); | ||
|
|
||
| // Check that the real BCL time zone behaves as reported in the issue: the transitions occur one millisecond early | ||
| var rules = bclZone!.GetAdjustmentRules(); |
There was a problem hiding this comment.
This isn't really related to this change, but I'm trying to understand why the null-forgiving operator is needed? From what I can see, the only useful use of this would be if we have an explicitly-nullable reference type (TimeZoneInfo?), but we don't?
(I'm looking at https://docs.microsoft.com/en-us/dotnet/csharp/nullable-references#nullable-annotation-context for reference.)
(Likewise a little bit below.)
There was a problem hiding this comment.
Are - it's not now. It was in an earlier version. The local variable isn't needed either! (Was just useful when debugging)
Transitions that would be represented as 24:00 in the IANA time zone database are represented as 23:59:59.999 in the Windows database. It makes sense to correct this as we have the facility to do so; the intent of the data is clearly "one millisecond later" even though that's not what the Windows rule actually says. (I don't think anyone would argue that the transition actually happens when the Windows rule says it does.) Fixes nodatime#1524.
|
Fixed the little problems; will merge on green. |
Codecov Report
@@ Coverage Diff @@
## master #1527 +/- ##
=======================================
Coverage 99.23% 99.23%
=======================================
Files 153 153
Lines 6812 6817 +5
=======================================
+ Hits 6760 6765 +5
Misses 52 52
Continue to review full report at Codecov.
|
- Document the potential for the removal of the `AllowPartiallyTrustedCallers` attribute to be a breaking change (if using .NET Framework and relying upon it, though it's still not actually clear to me how the 'new' security model works). - Document the (extremely minor) behaviour change introduced in nodatime/nodatime#1527.
- Document the potential for the removal of the `AllowPartiallyTrustedCallers` attribute to be a breaking change (if using .NET Framework and relying upon it, though it's still not actually clear to me how the 'new' security model works). - Document the (extremely minor) behaviour change introduced in nodatime/nodatime#1527.
Transitions that would be represented as 24:00 in the IANA time zone database are represented as 23:59:59.999 in the Windows database. It makes sense to correct this as we have the facility to do so; the intent of the data is clearly "one millisecond later" even though that's not what the Windows rule actually says. (I don't think anyone would argue that the transition actually happens when the Windows rule says it does.)
Fixes #1524.