Skip to content
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

Add support of multiple RRULE #131

Merged
merged 1 commit into from
Apr 3, 2024

Conversation

fabien-michel
Copy link
Collaborator

When an event got multiple RRULE.
Exemple:

RRULE:FREQ=WEEKLY;BYDAY=TH;COUNT=20
RRULE:FREQ=MONTHLY;BYDAY=2MO;COUNT=2

It was surprisingly easy to do, and surprising that it hadn't been done before. So I doubt I'm missing anything.

@fabien-michel fabien-michel marked this pull request as draft April 2, 2024 10:12
@niccokunzmann
Copy link
Owner

Uh nice!

Would you like to add a changelog entry, too?
Tell me when you are ready and I review it and merge. Looks good!

@fabien-michel fabien-michel force-pushed the support_multiple_rrule branch 5 times, most recently from af05d9b to 54068a0 Compare April 2, 2024 17:46
@fabien-michel
Copy link
Collaborator Author

Ok, I think you can review it.
Be really careful, all the tests are passing but I'm not really confident about changes I made. Especially the part modifying rdate conditions where I changed single-rrule UNTIL value to the max UNTIL value.

Copy link
Owner

@niccokunzmann niccokunzmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks really good!

I am pretty sure that this is better than before. Since this is a test-driven library, I am confident that you did not break anything! Thanks for testing, too, because that allows us to be confident!

recurring_ical_events.py Show resolved Hide resolved
recurring_ical_events.py Show resolved Hide resolved
@niccokunzmann niccokunzmann marked this pull request as ready for review April 3, 2024 10:23
@niccokunzmann niccokunzmann merged commit a9691ed into niccokunzmann:main Apr 3, 2024
7 checks passed
@niccokunzmann
Copy link
Owner

@fabien-michel Thanks a lot for the nice contribution. Feel free to add a new PR if you find that you would like to change the code a bit more!

@niccokunzmann
Copy link
Owner

I release this under v2.2.1.

@niccokunzmann
Copy link
Owner

Because of the clean work, I give you write access. With this, you can also start to release a new version, review and merge other PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants