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.

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

Remove PHP 5.2 support #264

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

szepeviktor
Copy link
Contributor

As per

* Requires PHP: 7.2

@mundschenk-at
Copy link
Owner

Yeah, that's one of the things I'll keep for 3.0, since strictly speaking, it's breaking binary compatibility. There are some other deprecations that need to be removed, but I don't want to do that in point-release as per SemVer.

@szepeviktor
Copy link
Contributor Author

For 3.0 here is a document on waving good by to PHP 4 and 5.
https://github.com/szepeviktor/small-project/blob/master/MAIN-FILE-PARTS.md

@mundschenk-at
Copy link
Owner

For 3.0 here is a document on waving good by to PHP 4 and 5.
https://github.com/szepeviktor/small-project/blob/master/MAIN-FILE-PARTS.md

Thanks. I'm not keen to inject a container for WordPress-derived constants (i.e. plugin file/path). What's the supposed purity benefit over a namespaced constant? (Over global constants, sure.)

@szepeviktor
Copy link
Contributor Author

:) I won't punish you if you don't follow every line of this document.
Namespaced constants are okay: they are separated from others.

@szepeviktor
Copy link
Contributor Author

One thing I've spotted: run_avatar_privacy();
It boots up everything before plugins_loaded unconditionally.

@mundschenk-at
Copy link
Owner

That's by design. It checks the requirements and sets up the controller instance for starting up the plugin components (which each set up their appropriate actions). The components are self-contained, so for example the deactivation_hook is set up in class Avatar_Privacy\Components\Setup. Now for Avatar Privacy, the components probably don't need to access any hooks before plugins_loaded, but I like to use the same structure for all my plugins and something else might need e.g. registered_post_type, so I prefer to keep the flexibility.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Apr 16, 2022

The components are self-contained

Yes that is an OOP thing.
In WordPress world you cannot do that: so things before and after plugins_loaded should be separated.
Actually there are very few things before it! My document linked above lists those things.
The example code in that repo implements before things in a procedural way mostly without objects.

"Cannot do that" means you go against WordPress when starting after things before plugins_loaded.

@mundschenk-at
Copy link
Owner

Not really, the action setup is just delegated to the run methods of the plugin components.

@szepeviktor
Copy link
Contributor Author

All right, this is not a boxing ring. 🥊
It is a forced marriage between WordPress and OOP 💍

@mundschenk-at
Copy link
Owner

As long as WordPress is WordPress, everything will be a compromise 😅

That said, I agree with you in principle: It's much better to enforce API contracts on a technical level than depend on "good behavior". There are a few instances where the Component::run methods do things that should be better no earlier than plugins_loaded (i.e. check for the existence of a Block Editor class - it could, or could have been, provided by a plugin instead of Core). I'll think about that some more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants