Skip to content

Added a method to get run dates in a date range - #41

Open
3p3r wants to merge 4 commits into
mtdowling:masterfrom
3p3r:master
Open

3p3r wants to merge 4 commits into
mtdowling:masterfrom
3p3r:master

Conversation

@3p3r

@3p3r 3p3r commented Nov 30, 2013

Copy link
Copy Markdown

Added a method to get run dates in a date range and some minor optimizations.

getRangeRunDates returns all run dates limited to a date range.
max(0, $total) doesn't need to be calculated in every iteration.
Comment thread src/Cron/CronExpression.php Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

null|string|DateTime, same for line below

@3p3r

3p3r commented Dec 1, 2013

Copy link
Copy Markdown
Author

Fixed input type in comments. Thanks @staabm

@mtdowling

Copy link
Copy Markdown
Owner

I like this feature. Can you add tests please?

Comment thread src/Cron/CronExpression.php Outdated

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can you add braces here? I don't like blocks without a wrapping brace

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Done :)

"cleaner code" by re aligning some comments and adding braces.
@3p3r

3p3r commented Dec 5, 2013

Copy link
Copy Markdown
Author

@mtdowling I will add tests too but with a couple of days delay. Sorry for that but it is my finals' week at school.

@viktoras25 viktoras25 mentioned this pull request Apr 24, 2014
@ghost

ghost commented Nov 18, 2014

Copy link
Copy Markdown

This is a really needed functionality.

peter279k pushed a commit to peter279k/cron-expression that referenced this pull request Nov 25, 2020
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.

5 participants