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

WHIP_VERSION constant logic is flawed #4

Open
schlessera opened this issue Feb 21, 2017 · 10 comments
Open

WHIP_VERSION constant logic is flawed #4

schlessera opened this issue Feb 21, 2017 · 10 comments

Comments

@schlessera
Copy link
Contributor

If I understood correctly, WHIP should work when installed in several different versions by different plugins. However, the way this is currently implemented will not work.

The constant WHIP_VERSION will only ever be defined once. If an old version gets loaded first, all other versions will use the version string of that version.

@atimmer
Copy link
Contributor

atimmer commented Feb 23, 2017

Do you have any idea how to solve this? I think we can never reliably discern which minor version is in use because we are always struggling with the autoloading order. We can discern major versions because we change the postfix of the classes for each major version.

I would opt for only returning the version in the file. Any location we need the version we just need to require that file then. We would drop the constant altogether.

Thoughts?

@schlessera
Copy link
Contributor Author

A mechanism I have seen that works with WP and uses the built-in mechanisms is to use the action priorities coupled to the version.

So, for example, you start at a high number with your hook priority, and the higher your version is, the more you subtract from the priority.

In this way, when you're loading several different versions at once, the one with the highest version will be triggered first. This one can then set a constant, that lets all the other versions know that stuff is already taken care of.

So, as an example:
WHIP 1.1 => init:9999
WHIP 2.2 => init:9988
WHIP 1.0 => init:10000
WHIP 2.0 => init:9990

The init:9988 hook will be triggered first, version 2.2. This one can then set constants to deactivate all the others.

@schlessera
Copy link
Contributor Author

Another way to do this is to use a static variable for the instance, and each time a new version loads, if it is more recent than the currently set instance, it replaces that instance. All the later code will always fetch that instance first before doing anything, so they always get the most recent version.

@schlessera
Copy link
Contributor Author

So, the object that a user instantiates would only be a proxy. The actual implementation set within that proxy could be replaced with more recent versions, without any of the consuming code noticing (provided that you remain backwards compatible).

@atimmer
Copy link
Contributor

atimmer commented Mar 11, 2017

For now, I have changed version.php to only return the version and not set a constant. This will fix the version reconciliation for major versions but not for minor&patch versions. This will do for now.

The plan is to fix minor versions later using Mozart.

@schlessera
Copy link
Contributor Author

I haven't checked the Mozart package in detail, but at a quick glance it looked to me like it string-replaced a given namespace with a prefixed namespace. As you don't have any namespace at all, that will likely not do anything.

@atimmer
Copy link
Contributor

atimmer commented Mar 14, 2017

@schlessera Coen is working on a classmap implementation that also works for PHP5.2: coenjacobs/mozart#4

@schlessera
Copy link
Contributor Author

Keep in mind that this will then load Whip multiple times in different versions with different namespaces. You still need to devise a way of only showing notices ones and communicating across versions to only let the newest one be active.

@schlessera
Copy link
Contributor Author

Ah, if I remember correctly, you use a global with the version number as key, right? That should probably be enough to start, just think of adding some kind of "killswitch" so that future versions can force older ones to not do anything.

@schlessera
Copy link
Contributor Author

You'll need to think of a different mechanism for the Facades when using Mozart, as the function cannot be autoloaded and will only be declared once. Within that function, you have class names and file paths that will point to one specific version (and probably not the correct one).

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

No branches or pull requests

2 participants