-
Notifications
You must be signed in to change notification settings - Fork 234
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
Make the time zone resolver plugable #732
Conversation
|
Codecov ReportAttention: Patch coverage is
❌ Your patch status has failed because the patch coverage (68%) is below the target coverage (80%). You can increase the patch coverage or adjust the target coverage. @@ Coverage Diff @@
## main #732 +/- ##
===================================
Coverage 66% 66%
===================================
Files 103 105 +2
Lines 4644 4645 +1
Branches 1152 1154 +2
===================================
+ Hits 3064 3066 +2
+ Misses 1144 1143 -1
Partials 436 436
|
Relates to the discussion in #606 (reply in thread) |
@@ -2291,7 +2291,7 @@ public void Yearly1() | |||
public void Bug2912657() | |||
{ | |||
var iCal = Calendar.Load(IcsFiles.Bug2912657); | |||
var localTzid = iCal.TimeZones[0].TzId; | |||
var localTzid = iCal.Events.First().Start.TzId; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
Thanks and thanks for the review! Having global static parameters is always a somewhat poor design but in this case it seems to be the best short term solution without refactoring the whole evaluation code. At least for our project its a viable workaround. Could/should be improved in v6. |
This PR implements several improvements related to resolving time zones: