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

mark factories as final #117

Merged
merged 1 commit into from Mar 7, 2016

Conversation

sandrokeil
Copy link
Member

As mentioned in #116

prolic added a commit that referenced this pull request Mar 7, 2016
@prolic prolic merged commit 980ddae into prooph:develop Mar 7, 2016
@prolic
Copy link
Member

prolic commented Mar 7, 2016

Thanks.

@codeliner
Copy link
Member

sorry for being late, but the factory changes definitely require a new major version. For example I extend from the factories to override the containerId. I can now achieve the same with the new "call static" feature but I need to migrate my application, so a new major version with a migration path is needed and we also need to align the docs!

Question: Is it possible to keep BC and mark for example Factory::containerId as deprecated first? Then we can remove the message with the next major release. Also the final keyword now prevents me from extending a factory and override its methods. I'm fine with that for next major but not for a minor release.

@prolic
Copy link
Member

prolic commented Mar 8, 2016

I am fine with a new major release, too.
We can implement the event store adapter strategies and allow php 7 only. would be a good bunch of new features, so let's work on that. I can start with adapter strategies and php7 migration in about 4 weeks, when i am in the philippines.

@codeliner
Copy link
Member

I won't have much time in March and April but hopefully have more time again with the beginning of Mai. But we can of course start (or better continue) with implementing the changes and if it takes a bit longer this time it is not really a problem as our current versions already have a rich feature set.

@sandrokeil
Copy link
Member Author

@prolic I think we should not bring a new major release at this time, maybe at the end of the year with PHP 7 only support. We should also make the upgrade path simply like ZF2 does. I can remove the final keyword.

@codeliner A possibility is to call containerId() in the constructor, but how to use it together with the parameter configId?

In event-store we have a drop in replacement.

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

Successfully merging this pull request may close these issues.

None yet

3 participants