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

DRAFT: Fix dst-transition-leap spreading to subsequent events #101

Closed
wants to merge 3 commits into from

Conversation

schreven
Copy link

No description provided.

@schreven schreven force-pushed the fix/recurrence-on-dst branch 2 times, most recently from 6dbb10b to 882a18e Compare April 19, 2024 12:33
@schreven schreven changed the title Fix dst-transition-leap spreading to subsequent events DRAFT: Fix dst-transition-leap spreading to subsequent events Apr 19, 2024
@schreven schreven force-pushed the fix/recurrence-on-dst branch 2 times, most recently from 7ff600f to d9f246d Compare April 22, 2024 10:02
Comment on lines +1292 to +1310
private function revertPastOffset(\DateTimeInterface $dateTime): \DateTimeInterface
{
if (!is_numeric($this->counter)) {
return $dateTime;
}

$previousIterationLeapOffset = $this->leapOffset[(int) $this->counter - 1] ?? null;

if (!is_int($previousIterationLeapOffset)) {
return $dateTime;
}

$modifiedDateTime = $dateTime->modify("- $previousIterationLeapOffset seconds");

unset($this->leapOffset[$this->counter - 1]);

return $modifiedDateTime;
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

if you are anyway always resetting the previous offset, why keep a list of them? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

Before my last commit, it was possible to call this method and "detectAndSaveDstOffset" multiple times per iteration. It's probably not needed anymore indeed

@schreven
Copy link
Author

I'm not satisfied with the approach altogether. It's error-prone and not really future proof. I'll close this and submit a bug report upstream

@schreven schreven closed this Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants