Skip to content

Add possibility to add additional dates to recurring events#45

Merged
stefanv merged 4 commits into
scientific-python:mainfrom
itrich:main
Jan 31, 2023
Merged

Add possibility to add additional dates to recurring events#45
stefanv merged 4 commits into
scientific-python:mainfrom
itrich:main

Conversation

@itrich

@itrich itrich commented Dec 20, 2022

Copy link
Copy Markdown
Contributor

This PR is related to #23. By adding also_on to the repeat property, further dates can be added to a recurring event.

This can be nicely combined with except_on in order to postpone a recurring event only within a specific week, e.g.:

  - summary: Monday meeting
    begin: 2022-12-05 10:00:00
    duration: { minutes: 60 }
    repeat:
      interval:
        weeks:1
      until: 2022-12-31
      except_on:
        - 2022-12-26 # Skip the second day of christmas...
      also_on:
        - 2022-12-28 10:00:00 # ...and meet on Wednesday instead

Kudos to @garloff for bringing up this idea! This feature saves us a lot of copy & paste work if we have to exceptionally reschedule a recurring event.

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

Merging #45 (102357d) into main (4205ffc) will increase coverage by 0.90%.
The diff coverage is 92.30%.

@@            Coverage Diff             @@
##             main      #45      +/-   ##
==========================================
+ Coverage   81.00%   81.90%   +0.90%     
==========================================
  Files           1        1              
  Lines         100      105       +5     
==========================================
+ Hits           81       86       +5     
  Misses         19       19              
Impacted Files Coverage Δ
yaml2ics.py 81.90% <92.30%> (+0.90%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@garloff

garloff commented Dec 20, 2022

Copy link
Copy Markdown

I love the feature, thanks for crediting me, @itrich!

@garloff garloff left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Yeah, staying within gh realm makes sense for a gh repo ...
/lgtm

@garloff garloff left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

/lgtm

Comment thread yaml2ics.py
@itrich

itrich commented Jan 10, 2023

Copy link
Copy Markdown
Contributor Author

@stefanv @jarrodmillman Would you mind spending some spare time on reviewing this PR? I checked the generated ICS with Google Calendar and Thunderbird. Both look fine and I couldn't spot any regression so far.

@stefanv

stefanv commented Jan 10, 2023

Copy link
Copy Markdown
Member

I'll take a look.

@stefanv stefanv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great, thanks! I left some minor feedback. In the test, maybe add multiple "also_on" values, just to be sure the list gets handled correctly.

Comment thread .pre-commit-config.yaml Outdated
Comment thread example/test_calendar.yaml Outdated
Comment thread yaml2ics.py Outdated
Comment thread yaml2ics.py Outdated
@stefanv

stefanv commented Jan 30, 2023

Copy link
Copy Markdown
Member

@itrich What do you think about the suggestions?

@bsipocz

bsipocz commented Jan 31, 2023

Copy link
Copy Markdown
Member

Oops, merging your other PR added a conflict, could you please rebase?

Signed-off-by: Eduard Itrich <eduard@itrich.net>
Signed-off-by: Eduard Itrich <eduard@itrich.net>
Signed-off-by: Eduard Itrich <eduard@itrich.net>
@itrich

itrich commented Jan 31, 2023

Copy link
Copy Markdown
Contributor Author

I rebased my branch onto main and merging should now be possible. Thank you for the kind review and allowing me to learn from you! 🙏

Signed-off-by: Eduard Itrich <eduard@itrich.net>
@itrich itrich requested a review from stefanv January 31, 2023 21:35
@stefanv stefanv merged commit b3279c5 into scientific-python:main Jan 31, 2023
@stefanv

stefanv commented Jan 31, 2023

Copy link
Copy Markdown
Member

Thanks @itrich!

@jarrodmillman jarrodmillman added this to the 0.2 milestone Feb 1, 2023
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.

6 participants