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 for months, years, and multi-digit increments #151

Merged
merged 3 commits into from
Jul 4, 2022

Conversation

aurisnoctis
Copy link

@aurisnoctis aurisnoctis commented Jul 2, 2022

Before: Recurrence periods in months m were re-calculated as weeks, which created an unexpected offset, e.g., 28 days instead of 1 month (which could have 30 or 31 days). As the number of months grows, the offset grows to a week or more. Recurrence periods in years y were recalculated as x years times 52 weeks, which created offsets as well.

Now: This is fixed, a month is a month and a year is a year, without using approximations in weeks.

@aurisnoctis
Copy link
Author

  • used the opportunity to correct a spelling mistake

@aurisnoctis aurisnoctis changed the title fix recurrence in months and years fix recurrence for months, years, and multi-digit increments Jul 3, 2022
@aurisnoctis
Copy link
Author

@oroulet
Copy link
Contributor

oroulet commented Jul 3, 2022

Great that you are looking at that!
I can merge directly if you want but I advice you to write some tests to check the cases you have fixed. Tasklib should have many tests already

@aurisnoctis
Copy link
Author

Great you like it!
Cannot promise when I could get tests ready.

@oroulet oroulet merged commit 83edf03 into QTodoTxt:master Jul 4, 2022
@oroulet
Copy link
Contributor

oroulet commented Jul 4, 2022

Ok. Merging now then

@aurisnoctis
Copy link
Author

👍 Thank you for being pragmatic. Writing tests for this is in my backlog.

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