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

Changes to acceptable values for bySetPos and byMonthDay to be consistent with iCalendar spec #430

Merged
merged 2 commits into from
Oct 10, 2022

Conversation

stevenpal
Copy link

About this Pull Request

  • What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
    • This change introduces two fixes to make the repeating rules for bySetPos and byMonthDay consistent with the iCalendar spec.
  • What is the current behavior? (You can also link to an open issue here)
    • bySetPos currently only allows a single number to be passed in.
    • bySetPos currently only allows values between -1 and 4.
    • byMonthDay currently only allows values between 1 and 31.
    • If multiple days are specified for byDay when also specifying bySetPos, only the first day passed in the array is used and the remaining ones are discarded.
  • What is the new behavior (if this is a feature change)?
    • bySetPos is now allowed to be passed in as both a number or an array.
    • bySetPos now allows values between -366 to -1 and 1 to 366.
    • byMonthDay now allows values between -31 to -1 and 1 to 31.
    • If multiple days are specified for byDay, they are all used in conjunction with bySetPos and none are discarded.
  • Does this PR introduce a breaking change? (What changes might users need to make in their application due to this PR?)
    • If you're relying on additional values of byDay being discarded when being used with bySetPos, that would be a change to existing behavior. Otherwise, this adds functionality rather than removing or changing it.
  • Other information:
    • The iCalendar spec describes the guidelines for bySetPos and byMonthDay: https://icalendar.org/iCalendar-RFC-5545/3-3-10-recurrence-rule.html
    • There are two tests failing off of the develop branch that are unrelated to these changes. They are related to support for Luxon and Day.js. I didn't try to fix those as I don't know much about those libraries.

Pull Request Checklist

  • My code follows the code style of this project
    • Run npm run lint to double check
  • My change requires a change to the documentation
    • I have updated the documentation accordingly
  • I have added tests to cover my changes
    • Run npm run test to run the unit tests and npm run coverage to generate a coverage report
  • All new and existing tests passed
  • My commit messages follow the commit guidelines

@sebbo2002 sebbo2002 merged commit b19e94b into sebbo2002:develop Oct 10, 2022
@sebbo2002 sebbo2002 mentioned this pull request Oct 10, 2022
github-actions bot pushed a commit that referenced this pull request Oct 10, 2022
# [3.6.0-develop.1](v3.5.2...v3.6.0-develop.1) (2022-10-10)

### Features

* **Event:** Update `bySetPos` and `byMonthDay` ([b19e94b](b19e94b)), closes [#430](#430)
@sebbo2002
Copy link
Owner

🎉 This PR is included in version 3.6.0-develop.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Oct 11, 2022
# [3.6.0](v3.5.2...v3.6.0) (2022-10-11)

### Features

* **Event:** Update `bySetPos` and `byMonthDay` ([b19e94b](b19e94b)), closes [#430](#430)
@sebbo2002
Copy link
Owner

🎉 This PR is included in version 3.6.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants