Skip to content

Conversation

@amirhshokri
Copy link
Contributor

@amirhshokri amirhshokri commented Jul 15, 2025

This PR updates the logic in the dailyAt() method to correctly handle time strings containing hours, minutes, and seconds (e.g., "10:20:30").

Previously, if the $time string contained more than two segments, the minutes value would incorrectly default to '0'.
By changing the condition from count($segments) === 2 to count($segments) >= 2, we ensure that the minutes value is preserved even when seconds are present. This does not change the existing behavior; seconds are still ignored exactly as before.

  • This could be done using count($segments) === 2 || count($segments) === 3, but I think the above method is more performant than using two count() calls.

@taylorotwell
Copy link
Member

I am closing this pull request because it lacks sufficient explanation, tests, or both. It is difficult for us to merge pull requests without these things because the change may introduce breaking changes to the framework.

Feel free to re-submit your change with a thorough explanation of the feature and tests - integration tests are preferred over unit tests. Please include it's benefit to end users; the reasons it does not break any existing features; how it makes building web applications easier, etc.

Thanks!

@amirhshokri amirhshokri deleted the 12.x-fix-extracting-hours-and-minutes-for-scheduler branch July 15, 2025 20:34
@amirhshokri amirhshokri changed the title [12.x] Fix hours and minutes extraction in scheduler dailyAt() method [12.x] Fix hours and minutes extraction in scheduler dailyAt() method Jul 16, 2025
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.

2 participants