Skip to content

Fix issue of not listing duplicate cron events#3175

Merged
danielbachhuber merged 5 commits into
wp-cli:masterfrom
ernilambar:3170-not-listed-duplicate-event
Jul 25, 2016
Merged

Fix issue of not listing duplicate cron events#3175
danielbachhuber merged 5 commits into
wp-cli:masterfrom
ernilambar:3170-not-listed-duplicate-event

Conversation

@ernilambar
Copy link
Copy Markdown
Member

@ernilambar ernilambar commented Jul 19, 2016 •
edited by danielbachhuber
Loading

Fixes #3170

@ernilambar
Copy link
Copy Markdown
Member Author

ernilambar commented Jul 20, 2016

Is there specific I need to do for WP 3.7? https://travis-ci.org/wp-cli/wp-cli/jobs/145729518#L304
I had assumed $hook-$sig-$time will give unique array key. I am not sure.

@danielbachhuber
Copy link
Copy Markdown
Member

danielbachhuber commented Jul 20, 2016

I had assumed $hook-$sig-$time will give unique array key. I am not sure.

Have you installed WP 3.7 locally and tested with it?

@ernilambar
Copy link
Copy Markdown
Member Author

ernilambar commented Jul 20, 2016 •
edited
Loading

No, I generally test with latest trunk in local. May be I need to setup WP 3.7 also.

@ernilambar
Copy link
Copy Markdown
Member Author

ernilambar commented Jul 21, 2016

@danielbachhuber I tried PR in WP 3.7 and its working as expected in local environment. Any idea what could be the cause of failed build?

@danielbachhuber
Copy link
Copy Markdown
Member

danielbachhuber commented Jul 21, 2016

I tried PR in WP 3.7 and its working as expected in local environment.

Can you share more details on what you tried?

@ernilambar
Copy link
Copy Markdown
Member Author

ernilambar commented Jul 21, 2016

I tried the PR in WP 3.7. Output is as expected. http://prntscr.com/bvr7z0
I dumped value from get_cron_events() function and all events are returned in array.

danielbachhuber
danielbachhuber reviewed Jul 21, 2016
View reviewed changes
Comment thread php/commands/cron.php Outdated
foreach ( $hook_events as $sig => $data ) {

$events["$hook-$sig"] = (object) array(
$events["$hook-$sig-$time"] = (object) array(
Copy link
Copy Markdown
Member

@danielbachhuber danielbachhuber Jul 21, 2016

Choose a reason for hiding this comment

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

Is this key actually used for anything, or could we always just add to the array?

Copy link
Copy Markdown
Member Author

@ernilambar ernilambar Jul 21, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

To make array key unique. I dont think it is used anywhere else.

Copy link
Copy Markdown
Member

@danielbachhuber danielbachhuber Jul 22, 2016

Choose a reason for hiding this comment

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

Right, but couldn't we instead do:

$events[] = (object) array(

@danielbachhuber danielbachhuber added this to the 0.24.0 milestone Jul 25, 2016
@danielbachhuber danielbachhuber merged commit 3a03638 into wp-cli:master Jul 25, 2016
@ernilambar ernilambar deleted the 3170-not-listed-duplicate-event branch July 25, 2016 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

bug command:cron-event command:cron-event-list

Projects

None yet

Milestone

0.24.0

Development

Successfully merging this pull request may close these issues.

2 participants

@ernilambar @danielbachhuber