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

[5.8] Allow hourlyAt() to accept array of integers #29173

Merged
merged 3 commits into from Jul 15, 2019

Conversation

@jrebs
Copy link
Contributor

commented Jul 15, 2019

This small PR expands the functionality of the ManagesFrequencies::hourlyAt() method to allow it to accept an array of integers instead of only a single integer. FrequencyTest class includes passing test for this addition.

This is a non-breaking enhancement because the hourlyAt() method does not typehint the offset value being passed in.

Benefit to users: finer control over granular execution schedules. The hourlyAt() method is not chainable, so in the follow example, the command would only be executed at the 45th minute:

$schedule->command('my:command')->hourlyAt(15)->hourlyAt(30)->hourlyAt(45)

With this PR, it is possible to declare whatever hourly granularity is needed for your use case:

$schedule->command(my:command')->hourlyAt([15, 30, 45]);

jrebs added some commits Jul 15, 2019

@jrebs jrebs changed the title Allow hourlyAt() to accept array of integers [5.8] Allow hourlyAt() to accept array of integers Jul 15, 2019

@taylorotwell taylorotwell merged commit e74508e into laravel:5.8 Jul 15, 2019

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/styleci/pr The analysis has passed
Details
@jrebs

This comment has been minimized.

Copy link
Contributor

commented on src/Illuminate/Console/Scheduling/ManagesFrequencies.php in 084546d Jul 15, 2019

Hi Taylor, thanks for merging my PR. Just so I have this correct in the future, is the intention that all of the param types in the docblock should be listed alphabetically? Even if some are application classes? For instance:

@param int|MySpecialClass

This comment has been minimized.

Copy link
Member

replied Jul 16, 2019

@jrebs no but it's more common to first list the array type in the framework I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.