ackintosh / ganesha

:elephant: A Circuit Breaker pattern implementation for PHP applications.
https://ackintosh.github.io/ganesha/
MIT License
593 stars 44 forks source link

1.2.0 Release backwards incompatibility #80

Closed madbriller closed 3 years ago

madbriller commented 3 years ago

Hello and thanks for your great work on this package. Does this package follow the https://semver.org/ standard?

If it does not, we may have to look into an alternative as it is hard to commercially rely on a package which will make breaking changes in a minor release. The breaking change being that the setConfiguration function has changed on the cache adapter interface to be setContext.

We have written a custom adapter for the Laravel framework and our solution to handle this is going to be to implement both functions. This would allow our adapter to interoperate with both versions of the package.

In 1.1.0, our setConfiguration call was empty and did nothing, but it seems the new setContext has a specific intent. It seems the changes are intended to clean up the cache after the time windows have expired. If we have an empty setContext function would we still see the old behaviour?

ackintosh commented 3 years ago

This package follows semver but unintentionally we have broke backwards compatibility in 1.2.1. I'm sorry, it is my fault.

Thank you for this issue, which let me know the backwards incompatibility.

I'll release a new version to fix the incompatibility in the adapter interface as soon as possible.


If we have an empty setContext function would we still see the old behaviour?

If your custom adapter inheriting a built-in adapter like below, I recommend to use the parent implementation.

class CustomAdapter extends \Ackintosh\Ganesha\Storage\Adapter\Memcached
{
    // Comment out to use the parent implementation
    // public function setContext(Ganesha\Context $context): void
    // {
    // }
}

If your custom adapter using no built-in adapter, it is no problem with an empty setContext.

madbriller commented 3 years ago

Thank you for your kind and timely response.

No worries on the backwards compatibility break, mistakes happen! What is more important is that you intend to follow semver which renews my confidence in the package :)

My implementation does not extend any of the built in adapters, as i'm allowing Laravel to choose which underlying cache driver to use. Thanks for the confirmation on the empty setContext, and thanks for the fix for the bc break.

If i get time i may submit a PR containing the Laravel cache adapter i have written.

ackintosh commented 3 years ago

@madbriller I have released v1.2.2 to fix the incompatibility but unfortunately this release does not completely fix the incompatibility.

An empty setContext is still required for custom adapter users.

https://github.com/ackintosh/ganesha/releases/tag/1.2.2