Conversation
class.jetpack.php
Outdated
|
|
||
| elseif ( site_url() && false === strpos( site_url(), '.' ) ) { | ||
| $development_mode = true; | ||
| elseif ( $site_url = site_url() ) { |
There was a problem hiding this comment.
I'm not sure why we even need to check if site_url() returns something (something would be pretty broken if it didn't), but it's possible it's this way for a reason.
There was a problem hiding this comment.
I don't know—it's original to the function as committed to Jetpack eff10c9
There was a problem hiding this comment.
Yeah, doesn't hurt to leave it.
|
Too many comparisons on the same line, like I'd much rather see this code block changed to something resembling ... $site_url = site_url();
if ( ! empty( $site_url ) || ! wp_in( '.', $site_url ) ) {
$development_mode = true;
}We ship |
Agreed, but this is an inside an
If the load order ever changes, that could be bad :) |
eliorivero
left a comment
There was a problem hiding this comment.
Added a comment to fix the indentation that was previously ok.
To further simplify this and avoid the $development_mode assignment, this could be reduced to
return apply_filters( 'jetpack_development_mode',
( defined( 'JETPACK_DEV_DEBUG' ) && JETPACK_DEV_DEBUG ) || ( false === strpos( (string) site_url(), '.' ) )
? true
: false
);If it feels too complex, maybe it could be rewritten as:
if ( defined( 'JETPACK_DEV_DEBUG' ) ) {
$development_mode = JETPACK_DEV_DEBUG;
} elseif ( $site_url = site_url() ) {
$development_mode = false === strpos( $site_url, '.' );
} else {
$development_mode = false;
}the idea is to initialize $development_mode only once.
class.jetpack.php
Outdated
| elseif ( site_url() && false === strpos( site_url(), '.' ) ) { | ||
| $development_mode = true; | ||
| elseif ( $site_url = site_url() ) { | ||
| $development_mode = false === strpos( $site_url, '.' ); |
There was a problem hiding this comment.
Can you fix the indentation here? It was changed from a tab to spaces (and more than needed). The spacing before the elseif could also be removed.
There was a problem hiding this comment.
Yep, good catch. Fixed the indents and spacing.
Instead of calling the function twice, which is a waste, assign the value to a variable and use that value to check whether we're on a tld-less domain or not.
6e78b4c to
8b75eb6
Compare
I don't think we gain that much from this change. |
* Changelog: update stable tag and move changelog to changelog.txt Also remove old releases from readme.txt to keep the changelog tab short. * Changelog: add #5883 Also update the filter's docblock to match new version. * Changelog: add #5938 * Changelog: add #6298 * Changelog: add #3405 * Changelog: add #5941 * Changelog: add #6239 * Changelog: add #6281 * Changelog: add #6303 * Changelog: add #6018 * Changelog: add #6300 * Changelog: add #6296 * Changelog: add #6130 * Changelog: add #6292 * Readme: remove extra "on". * Changelog: add #6307 * Changelog: add #3297 * Changelog: add #6275 * Changelog: add #6321 * Changelog: add #6297 * Readme: update the support forum link anchor. Anchor changed when WordPress.org forums were updated to bbPress 2 * Readme: update list of a12s, it wasn't up to date anymore! * Changelog: add #6338 * Changelog: add #6337 * Changelog: add #6335 * Changelog: add #6333 * Testing List: first version of the 4.7 testing list. * Changelog: add #6332 * Changelog: add #6325 * Changelog: add #6326 * Changelog: add #6339 * Changelog: add #6342 * Changelog: add #6343 * Changelog: add #6346 * Changelog: add #6347 * Changelog: add #6279 * Changelog: add #6306 * Changelog: add #6312 * Changelog: add #6316 * Changelog: add #6171 * Changelog: add #6317 * Changelog: add #6246 * Changelog: add #6263 * Changelog: add #4220 * Changelog: add #5888 * Changelog: add #3406 * Changelog: add #3637 * Changelog: add #6320 * Changelog: add #5992 * Changelog: add #6322 * Changelog: add #6324 * Changelog: add #6352 * Changelog: add #6355 * Changelog: add #6360 * Changelog: add #6362 * Changelog: add #6369, #6382 * Changelog: add #6370 * Changelog: add #6375 * Changelog: add #6383 * Changelog: add #6384 * Changelog: add #6386 * Changelog: add #6395 * Changelog: add #6403 * Changelog: add #6406 * Changelog: add #6418 * Changelog: add #6419 * Changelog: add #6434 * Changelog: add #6446 * Changelog: add #6006 * Changelog: add #6096 * Changelog: add #6399 * Changelog: fix typo. @see #6331 (comment) * Changelog: add #6440 * Changelog: add #6443 * Changelog: add #6445 * Changelog: add #6463 * Changelog: add #6468 * Changelog: add #6471 * Changelog: add #6474 * Changelog: add #6480 * Changelog: add #6497 * Changelog: add #6499 * Changelog: add #6514 * Changelog: add #6267 * Changelog: add #5940 * Changelog: add #6492 * Changelog: add #5281 * Changelog: add #6327 * Changelog: add #6451 * Changelog: add #6525 * Changelog: add #6530
In
Jetpack::is_development_mode, we current call thesite_url()function twice on the same line. This is a waste since the value is unlikely to change between those two calls.Changes proposed in this Pull Request:
Assign the value of
site_url()to a variable and uses that value to check whether we're on a tld-less domain or not.Testing instructions:
JETPACK_DEV_DEBUGconstant is not set.localhost, verify that the value ofJetpack::is_development_mode()istrue.localhost.dev, verify that the value ofJetpack::is_development_mode()isfalse.