-
Notifications
You must be signed in to change notification settings - Fork 247
RRULE evaluation: Ignore BYtime if DTSTART is date-only #619
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
RRULE evaluation: Ignore BYtime if DTSTART is date-only #619
Conversation
…nly, as required by RFC 5545.
|
axunonb
left a comment
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.
Nicely commented,, important fix, tnx
|
@minichma Feel free to merge the 2 approved PRs after considering whether the remarks are useful or not. The only thing I'm pretty much convinced by now is: we need more coverage of use cases from the wild, which results in more effort than code coverage. Thanks for the structured approach you already started. |
|
Regarding test cases for this fix I was running this based on #617. I think we should consider merging #617 and introducing our own test files in the same format. This would allow us adding new tests very easily and also to update test cases from the libical project. Then we'd also have specific coverage for this PR. |
I'm pretty sure it would. Most important would be that the overall coverage would increase over time. Imposing high quality figures would only be helpful if we also have a reasonable path to get there. |



According to RFC 5545, when evaluating an RRULE based on a DTSTRT of type DATE (i.e. date-only), any BY[SECOND|MINUTE|HOUR] MUST be ignored. This is implemented by this PR.
This fixes the case
DTSTART:20241018, RRULE:FREQ=DAILY;BYMINUTE=1,2,3,4;INTERVAL=2;COUNT=3of #618.