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

Replace Hungarian notation with PHP type delarations #35

Open
Rumperuu opened this issue Feb 23, 2021 · 3 comments · May be fixed by #191
Open

Replace Hungarian notation with PHP type delarations #35

Rumperuu opened this issue Feb 23, 2021 · 3 comments · May be fixed by #191
Assignees
@Rumperuu
Labels
documentation Improvements or additions to documentation good first issue Good for newcomers security Anything with security implications
Milestone
3.0

Comments

@Rumperuu
Copy link
Collaborator

Rumperuu commented Feb 23, 2021 •
edited

Currently, variables are named using Hungarian notation (e.g., $l_str_foo). This is both harder to read and provides no programmatic type guarantees (as as variable with $l_str in the name can just as easily be assigned any type of value, not just a string).

This should be refactored to use the type declarations introduced in PHP 7, ensuring that errors will be thrown if implicit type coercion is detected.

Note: I'm not sure if WP Plugins have to target a specific version of PHP, in which case we may not have access to type declarations

Related: #36
@Rumperuu Rumperuu added documentation Improvements or additions to documentation good first issue Good for newcomers security Anything with security implications labels Feb 23, 2021
@Rumperuu Rumperuu mentioned this issue Feb 23, 2021
Open
@Rumperuu Rumperuu added invalid This doesn't seem right and removed invalid This doesn't seem right labels Feb 23, 2021
@Rumperuu
Copy link
Collaborator Author

https://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/

@lolzim
Copy link
Contributor

lolzim commented Mar 12, 2021

According to WP, 9 months ago 25% of WP was running on PHP 5.x:

image

I have no 5.6 instance but those that have updated Wordpress as far as possible should now see a huge warning saying something like "Get a decent PHP version NOW".
And recent WP release should not install/update on 5.6; from what I know 7.2 is teh minimal PHP version for current WP version.

That said, yes, this notation sucks.

Rumperuu added a commit that referenced this issue Apr 14, 2021
@Rumperuu
style: rename production flag variable
b824a44 
Renames the current production flag variable to be shorter,
non-Hungarian (presaging changes to come in #35) and not specifically
CSS_-related.

See #80
@markcheret markcheret added this to the 2.7.1 milestone Apr 16, 2021
@markcheret
Copy link
Owner

we currently have PHP 7.0 as required. It sounds reasonable to me.

This was referenced Apr 16, 2021
Closed
Merged
@Rumperuu Rumperuu modified the milestones: 2.7.1, 2.8 Apr 17, 2021
This was referenced Apr 27, 2021
Merged
Merged
Merged
@Rumperuu Rumperuu linked a pull request Aug 2, 2021 that will close this issue
Draft
@Rumperuu Rumperuu linked a pull request Aug 2, 2021 that will close this issue
3.0 #191
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Rumperuu Rumperuu

Labels
documentation Improvements or additions to documentation good first issue Good for newcomers security Anything with security implications
Projects
None yet
Milestone
3.0
Development

Successfully merging a pull request may close this issue.

3.0 markcheret/footnotes
3 participants
@Rumperuu @markcheret @lolzim