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

Fix recurrence iteration where there is a BYMONTHDAY rule greater than the number of days in the start month. #531

Merged
merged 1 commit into from
Sep 14, 2023

Conversation

darktrojan
Copy link
Collaborator

This gets tricky. If the starting month doesn't have an occurrence (e.g. June has no 31st day), we need to find the next month that does. It's possible to have a sequence of 3 months without as 31st at an interval of 2 (February, April, June) so we might need to look for the next month three times.

(In the Gregorian calendar it IS possible to miss 7 occurrences if the interval is 12 and the start is a February late in a century, but that's absurd and I will be dead by then.)

@coveralls
Copy link

Pull Request Test Coverage Report for Build 3019342029

  • 0 of 9 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.007%) to 95.439%

Totals Coverage Status
Change from base Build 3012316465: -0.007%
Covered Lines: 2935
Relevant Lines: 3026

💛 - Coveralls

@coveralls
Copy link

coveralls commented Sep 9, 2022

Pull Request Test Coverage Report for Build 3416378920

  • 20 of 20 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.003%) to 98.141%

Totals Coverage Status
Change from base Build 3275454147: 0.003%
Covered Lines: 9214
Relevant Lines: 9373

💛 - Coveralls

@darktrojan
Copy link
Collaborator Author

Apart from waiting for the ES6 work, is this otherwise okay? I'd like to get it heading towards shipping in Thunderbird.

@dilyanpalauzov
Copy link
Contributor

This is very similar to #418. I have not gone in details, but for me it seems both changesets approach problems with BYMONTHDAY in a different ways.

@mschroeder
Copy link
Collaborator

This is very similar to #418. I have not gone in details, but for me it seems both changesets approach problems with BYMONTHDAY in a different ways.

It is and together with the the already merged PR #530, the PR #418 is no longer needed as its development stalled years ago.

@dilyanpalauzov
Copy link
Contributor

… the PR #418 is no longer needed as its development stalled years ago.

Why don’t you close #418 then?

@mschroeder
Copy link
Collaborator

mschroeder commented Sep 28, 2022

… the PR #418 is no longer needed as its development stalled years ago.

Why don’t you close #418 then?

I do not like how you phrase things in your comments here in the last weeks. It sounds rather pushy and impolite.

I myself will close the other PR when I have time to cross check if the test cases there are worth keeping in addition to the ones added in the other two PRs. If anybody wants to close it earlier, he could do so.

moz-v2v-gh pushed a commit to mozilla/releases-comm-central that referenced this pull request Oct 3, 2022
…le greater than the number of days in the start month. r=#thunderbird-reviewers,mkmelin

kewisch/ical.js#531

Differential Revision: https://phabricator.services.mozilla.com/D158472

--HG--
extra : rebase_source : bdc9ddfc44d0eed8544fc8a01b8ec2e3ef345cd1
extra : amend_source : 98759bbffa6371ecf3ec3e46699cd13daaab898b
@leftmostcat leftmostcat merged commit 3754b83 into kewisch:main Sep 14, 2023
@darktrojan darktrojan deleted the overflow-bymonthday branch April 18, 2024 00:58
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