Port yaml2ics to ics 0.8.0 - #4
Conversation
stefanv
left a comment
There was a problem hiding this comment.
Thanks @rkdarst. I think these are good improvements! Take a look at the comments, and we can merge when you're ready.
My ultimate test for these ICS files is to import them into a test calendar on Google Calendar. If that works, and the meetings appear where they're supposed to, that satisfies our use-case.
| assert 'DTEND' not in event | ||
| assert 'RRULE:FREQ=YEARLY;UNTIL=20300422T000000Z' in event | ||
| assert 'DTEND' not in event.serialize() | ||
| assert 'RRULE:FREQ=YEARLY;UNTIL=20300422T000000' in event.serialize() |
There was a problem hiding this comment.
I guess it doesn't matter much whether the Z is there or not for day events?
There was a problem hiding this comment.
I can't say I know. We should do a live test.
- Various changes throughout, see scientific-python#3
|
"Works for me", possibly still some underlying issues but they can be fixed as they are found... "Import to google calendar or outlook calendar" is also exactly my final use case. I have not tested that with these changes yet. |
|
Roadmap for v0.8: ics-py/ics-py#245 It looks like an official alpha release is coming soon! I am mainly adding this here so @N-Coder can see we might be "daring enough to use the bleeding-edge version as tester." |
- Make all functions return objects, until the very end
- Rename event_ics_from_yaml → event_from_yaml
- Rename events_to_calendar_ics → events_to_calendar
- Add a test of whole-calendar events
- This required splitting out some of the previous __main__ script
to a separate function. I am not entirely sure if I made the
right choice, but it should be easy to adjust later.
|
@rkdarst I see you force pushed to the branch. Github unfortunately doesn't notify us of these events, so if you are ready to proceed just ping us with a comment or request a review. Thank you! |
|
I looked at the changes / comments and it seems like the only task remaining is to test this on Google calendar. |
|
I was able to import it into google calendar (see https://rkdarst.github.io/aaltoscicomp-calendar-2/ ). It does import to google calendar, I am making one test of updating it to ensure it still works (a previous test did work). I am wondering what else is remaining to do... though, I am sure it will need continued bugfixing and improvements. Somehow I thought that force-pushing did send notifications. Or maybe it doesn't because this is a draft PR? Known problems: my experimental timezone support (not included in this PR) doesn't work with all-day events. I filed an upstream issue about it, but it doesn't need to delay this I think. |
|
Currently we are able to do day events and schedule events for certain times/durations in the TZ of our choice (Pacific). As long as we can continue doing that, I am happy to merge for now. |
|
Related question: How much do you value reading in multiple yaml files, and producing only one calendar out of it? That made some of the development a bit more difficult, for the next timezone part I was working on. |
|
This is quite important to us, as we would like to create a hierarchy of calendars. I.e., a top-level calendar that contains multiple smaller calendars. |
|
@rkdarst I assume this supports all-day-events (as per https://github.com/scientific-python/yaml2ics/pull/4#issuecomment-934108501 comment)? If so, are we ready to merge? |
|
Yes, this particular PR should have nothing changed about all-day events. The issue mainly comes up when you try to declare a timezone for a whole file (but is now worked around upstream, well, once a PR is merged). So, I think you should have no problems now. I guess it's worth running one to check. |
|
can someone ensure tests pass outside of my environment? |
|
I tested it, and it works! |
This is my attempt at updating to ics 0.8.0. I probably haven't fixed everything, tests don't pass, but I thought I would submit the PR for reference, rather than let this work go to waste. I fully expect that this might be discarded and we go with a fully other plan after learning how ics is supposed to work in the new version. Or possibly even finding another solution?
Nothing here is validated to be correct (other than working and passing some, but not all, tests) - don't act on this without careful validation!