Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 41 additions & 0 deletions WordPress/Docs/WP/GlobalVariablesOverrideStandard.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<?xml version="1.0"?>
<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
title="Global Variables Override"
>
<standard>
<![CDATA[
WordPress global variables should not be overridden. This includes variables imported with
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my opinion, just saying "WordPress global variables should not be overridden." is enough and second phrase is not necessary, but other reviewers might disagree with me, so feel free to leave it for now if you prefer.

the `global` statement within functions, variables assigned via the `$GLOBALS` superglobal array,
and variables assigned in the global namespace.

A few select WordPress global variables like `$content_width` and `$wp_cockneyreplace` are
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since those are the two only WP global variables that can be overriden, I think this paragraph could be rephrased to remove the "like".

specifically intended to be overridden by themes/plugins and are exempt from this rule.
]]>
</standard>
<code_comparison>
<code title="Valid: Different variable name.">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this title. Maybe Valid: Using custom variable/key names instead of WordPress globals. or Valid: Using custom variable/key names. is a bit better and more descriptive? Happy to hear other suggestions you might have.

The concern that I have with Valid: Different variable name. is that it reads a bit vaguely and doesn't clearly communicate what makes the examples below valid.

<![CDATA[
$my_query = new WP_Query( $args );
Copy link
Collaborator

Choose a reason for hiding this comment

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

Following the pattern that I suggested below, I think that here you can highlight the $my_query variable. Typically, the <em> tags are used to highlight both the "dos" as well as the "don'ts".

]]>
</code>
<code title="Invalid: Global variable overridden.">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<code title="Invalid: Global variable overridden.">
<code title="Invalid: WordPress global variable overridden.">

I think it is important to mention that this is about WordPress globals and not any globals.

<![CDATA[
<em>global $wp_query;</em>
<em>$wp_query = new WP_Query( $args );</em>
Copy link
Collaborator

Choose a reason for hiding this comment

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

For this code example, I would suggest highlighting with the <em> tag only the $wp_query variable instead of both lines of the code example.

]]>
</code>
</code_comparison>
<code_comparison>
<code title="Valid: Custom Superglobal variable.">
Copy link
Collaborator

Choose a reason for hiding this comment

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

The superglobal is $GLOBALS and not $GLOBALS['my_data']. I believe the titles in this code_comparison block need to be rephrased. Alternatively, I suggest considering using just a single code_comparison block and presenting two code examples. One with a global override using global $wp_variable_name and another usigin $GLOBALS['wp_variable_name'].

Copy link
Author

Choose a reason for hiding this comment

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

I ended up simplifying it.

<![CDATA[
$GLOBALS['my_data'] = array( 'key' => 'value' );
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my remarks above regarding the <em> tag.

Copy link
Collaborator

Choose a reason for hiding this comment

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

This line has 49 characters and the maximum is 48.

]]>
</code>
<code title="Invalid: WordPress Superglobal variable overridden.">
<![CDATA[
<em>$GLOBALS['wp_query'] = new WP_Query( $args );</em>
]]>
</code>
</code_comparison>
</documentation>
Loading