Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Prefix Your Globals #1726

Merged
merged 8 commits into from
Jun 2, 2022
Merged

Prefix Your Globals #1726

merged 8 commits into from
Jun 2, 2022

Conversation

Ipstenu
Copy link
Contributor

@Ipstenu Ipstenu commented Jun 20, 2019 •
edited by jrfnl
Loading

Globals need to be uniquely and distinctly named in order to prevent conflicts with other plugins and themes.

Prefixes may not use terms designated for WordPress.

Two Letter Prefixes are also bad (80+ THOUSAND PLUGINS, wanna know how many have the same two-letter initials? Ouch)

Related #1722

jrfnl
jrfnl requested changes Jun 20, 2019
View reviewed changes
Copy link
Member

@jrfnl jrfnl left a comment

Choose a reason for hiding this comment

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

Hi Mika, I've had a look and left some comments.

Also: should it be mentioned that there is an error for prefixes which use characters outside of the characters which PHP allows for constructs ?

WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated
</code_comparison>
<standard>
<![CDATA[
Using any prefixes designated for WordPress are not permitted, even if WordPress has not used the define. This includes all defines such as _() (underscores), __() double underscores, wp_, and wordpress_
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Using any prefixes designated for WordPress are not permitted, even if WordPress has not used the define. This includes all defines such as _() (underscores), __() double underscores, wp_, and wordpress_
Using any prefixes reserved for WordPress are not permitted, even if WordPress has not started using the prefix (yet). This includes all defines such as _() (underscores), () double underscores, wp, and wordpress

Copy link
Member

Choose a reason for hiding this comment

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

This includes all defines such as _() (underscores), () double underscores, wp, and wordpress

What do you mean to say by this ? () are not characters allowed in prefixes as they are not allowed in PHP namespace/class/function/constant names etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ate __() for some reason...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added in second example.

WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated Show resolved Hide resolved
WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated Show resolved Hide resolved
GaryJones
GaryJones reviewed Jun 23, 2019
View reviewed changes
WordPress/Docs/NamingConventions/PrefixAllGlobalsStandard.xml Outdated
</code_comparison>
<standard>
<![CDATA[
Using any prefixes reserved for WordPress are not permitted, even if WordPress has not used the define. This includes all defines such as _() (underscores), __() double underscores, wp_, and wordpress_
Copy link
Member

Choose a reason for hiding this comment

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

Possible to add a link here to where the full list of WordPress-reserved prefixes are documented?

Copy link
Member

Choose a reason for hiding this comment

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

Good question. Got a link ?

Ipstenu and others added 8 commits May 17, 2022 14:20
@Ipstenu @jrfnl
Prefix Your Globals
cffede6
@Ipstenu @jrfnl
Prefix Your Globals (fix valid/invalid)
eada4c6
@Ipstenu @jrfnl
Fix code compareision, made better examples
55496ca
@Ipstenu @jrfnl
Explain what globals are
2e819b9
@Ipstenu @jrfnl
End of line
2d95295
@Ipstenu @jrfnl
Better examples
1fd4117
@Ipstenu @jrfnl
initials
6b814c9
@jrfnl
Docs/PrefixAllGlobals: various fixes after review
5c5eb5b 
* Use consistent indentation in the file.
* Expand the base explanation.
* Improve the valid/invalid descriptions.
* Use `<em>` tags to mark valid/invalid code.
* Expand the code samples.
* Add a code comparison demonstrating how namespaces affect the prefixes.
* Remove the unclear sentence regarding reserved prefixes.
* Clarify the three character minimum length rule.
@jrfnl
Copy link
Member

jrfnl commented May 31, 2022

I've rebased the PR and added one commit to make the fixes needed and other improvements. Let's get this merged!

For whomever will merge this: probably squash/merge would be a good idea.

@jrfnl jrfnl added this to the 3.0.0 milestone May 31, 2022
@dingo-d dingo-d merged commit cfdb6d1 into WordPress:develop Jun 2, 2022
@KamilBaczkowski KamilBaczkowski mentioned this pull request Oct 18, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GaryJones GaryJones GaryJones left review comments

@dingo-d dingo-d dingo-d approved these changes

@jrfnl jrfnl jrfnl approved these changes

Assignees
No one assigned
Labels
Type: Documentation
Projects
None yet
Milestone
3.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@Ipstenu @jrfnl @GaryJones @dingo-d