mundschenk-at / avatar-privacy

GDPR-conformant avatar handling for WordPress
GNU General Public License v2.0
17 stars 3 forks source link

Remove PHP 5.2 support #264

Open szepeviktor opened 2 years ago

szepeviktor commented 2 years ago

As per https://github.com/mundschenk-at/avatar-privacy/blob/30026c92fc10784c55f3222367024c39386b84f6/avatar-privacy.php#L34

mundschenk-at commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

:) 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 commented 2 years ago

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

mundschenk-at commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

szepeviktor commented 2 years ago

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

mundschenk-at commented 2 years ago

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.