Skip to content

Modules: Confirm array is present#9245

Merged
oskosk merged 2 commits into
masterfrom
kraftbj-patch-5
Apr 19, 2018
Merged

Modules: Confirm array is present#9245
oskosk merged 2 commits into
masterfrom
kraftbj-patch-5

Conversation

@kraftbj
Copy link
Copy Markdown
Contributor

@kraftbj kraftbj commented Apr 6, 2018

After d2fc8b9 , some sites were seeing warnings:

Warning: in_array() expects parameter 2 to be array, null given in 
/home/public_html/wp-content/plugins/jetpack/class.jetpack.php on 
line 1518

It appears we should be setting a default array('free') if there are no headers present, so I am not sure where this failure is coming from. This would prevent the warning.

cc: @gravityrail

After d2fc8b9 , some sites were seeing warnings:

Warning: in_array() expects parameter 2 to be array, null given in 
/home/public_html/wp-content/plugins/jetpack/class.jetpack.php on 
line 1518 

It appears we should be
@kraftbj kraftbj added Bug When a feature is broken and / or not performing as intended General [Status] Needs Review This PR is ready for review. labels Apr 6, 2018
@kraftbj kraftbj added this to the 6.0.1 milestone Apr 6, 2018
@kraftbj kraftbj requested a review from a team as a code owner April 6, 2018 16:35
oskosk
oskosk previously requested changes Apr 6, 2018
Comment thread class.jetpack.php Outdated
// get available features
foreach ( self::get_available_modules() as $module_slug ) {
$module = self::get_module( $module_slug );
if ( ! isset( $module ) || ! is_array( $module ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing parens closing the whole if condition here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🤦‍♂️ Thank you!

@oskosk oskosk modified the milestones: 6.0.1, 6.1 Apr 6, 2018
@oskosk
Copy link
Copy Markdown
Contributor

oskosk commented Apr 6, 2018

@kraftbj I pushed a commit adding the missing parens so tests can run again

@oskosk oskosk dismissed their stale review April 6, 2018 17:30

issue was addressed

@gravityrail
Copy link
Copy Markdown
Contributor

Good catch! Sorry about the warning.

Copy link
Copy Markdown
Contributor

@gravityrail gravityrail left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Copy Markdown
Contributor

@oskosk oskosk left a comment

Choose a reason for hiding this comment

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

LGTM

@oskosk oskosk added [Status] Needs Changelog [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Apr 19, 2018
@oskosk oskosk merged commit acc1a3d into master Apr 19, 2018
@oskosk oskosk deleted the kraftbj-patch-5 branch April 19, 2018 12:30
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Apr 19, 2018
oskosk added a commit that referenced this pull request Apr 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug When a feature is broken and / or not performing as intended General

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants