Enable NRT for CalendarComponents
#764
Conversation
Codecov ReportAttention: Patch coverage is ❌ Your patch status has failed because the patch coverage (26%) is below the target coverage (80%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #764 +/- ##
===================================
- Coverage 67% 67% -0%
===================================
Files 104 104
Lines 4496 4492 -4
Branches 1108 1111 +3
===================================
- Hits 2998 2994 -4
+ Misses 1071 1068 -3
- Partials 427 430 +3
🚀 New features to boost your workflow:
|
| // Standard | ||
| var standardIntervals = intervals.Where(x => x.Savings.ToTimeSpan() == new TimeSpan(0)).ToList(); | ||
| var latestStandardInterval = standardIntervals.OrderByDescending(x => x.Start).FirstOrDefault(); | ||
| var latestStandardInterval = standardIntervals.OrderByDescending(x => x.Start).First(); |
There was a problem hiding this comment.
standardIntervals is not guaranteed to be non-empty here, so .First() might throw.
There was a problem hiding this comment.
Right, then we could just return early with the current vTimeZone
|
|
||
| private DateTimeZone _nodaZone; | ||
| private string _tzId; | ||
| private DateTimeZone _nodaZone = DateTimeZone.Utc; // must initialize |
There was a problem hiding this comment.
In a future (v6) version, the Noda tz should not be part of this model object at all.
There was a problem hiding this comment.
Yes, agree. The initialization is a NRT thing, and shouldn't have side effects.
Ical.Net/Evaluation/TodoEvaluator.cs
Outdated
| private void DetermineStartingRecurrence(RecurrencePattern recur, ref CalDateTime referenceDateTime) | ||
| { | ||
| if (recur.Count.HasValue) | ||
| if (recur.Count.HasValue && Todo.Start != null) |
There was a problem hiding this comment.
This whole EvaluateToPreviousOccurrence that this method belongs to, is pretty weird. Not sure, what the right behavior in this case is. Probably just return if Todo.Start is null. Going into the else branch if its null probably isn't correct.
There was a problem hiding this comment.
Yep, returning directly is better then.
|
No description provided.