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

Calling parent constructor correctly for 2017.10 / PHP7 compatibility #212

Merged
merged 2 commits into from Dec 7, 2017

Conversation

6 participants
@pkamps
Copy link
Collaborator

commented Nov 9, 2017

It's a follow up of
ezsystems/ezpublish-legacy#1233

Currently ezfind will through a fatal error search for an object.

Testing instructions

  • in the admin interface do a search that will return result entries
  • without this patch it will result in a PHP fatal error
  • with this patch it will just show the result objects and there is no PHP fatal error

@andrerom andrerom changed the title Calling parent constructor correctly Calling parent constructor correctly for 2017.10 / PHP7 compatibility Nov 9, 2017

@andrerom

This comment has been minimized.

Copy link
Member

commented Nov 9, 2017

Is this or other cases common? Afaik @jeromegamez was adding some BC methods around, so maybe worth it if we add one for this issue as well.

@andrerom

This comment has been minimized.

Copy link
Member

commented Nov 14, 2017

@pkamps I guess we might need to add bump in requirement for legacy >= 2017.10 with this change or similar before we merge, to avoid issues for people getting this once we tag but on a older legacy. WDYT @emodric?

@emodric

This comment has been minimized.

Copy link
Contributor

commented Nov 14, 2017

Might be wise, yes.

@peterkeung peterkeung requested review from peterkeung and removed request for peterkeung Dec 6, 2017

@peterkeung

This comment has been minimized.

Copy link
Collaborator

commented Dec 6, 2017

+1

@gggeek

This comment has been minimized.

Copy link
Collaborator

commented Dec 6, 2017

+1 with the change in requirements

@pkamps

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 7, 2017

I assume that bump has to happen in the composer.json file in this extension?
I must admit that I don't have much experience with composer - maybe you can help me out here?

The composer.json requires the "ezsystems/ezpublish-legacy-installer" - but that's different to "ezsystems/ezpublish-legacy", right? So I'm not sure if it makes sense to require a min version for "ezsystems/ezpublish-legacy-installer" - or maybe we need to change the requirements all together?

@emodric

This comment has been minimized.

Copy link
Contributor

commented Dec 7, 2017

You can add a require for ezsystems/ezpublish-legacy in version >=2017.10. No need for removing the existing ones.

@emodric

emodric approved these changes Dec 7, 2017

@andrerom andrerom merged commit 1d20767 into ezsystems:master Dec 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.