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

Rename Object to JsonLDObject (Fix PHP 7.2) #92

Merged
merged 4 commits into from Nov 18, 2018

Conversation

Projects
None yet
7 participants
@whikloj
Contributor

whikloj commented Apr 10, 2018

Similar to #79 but doesn't switch back to use stdClass() but instead changes the old Object name to JsonLDObject. Also fixes two locations where arrays were assumed for uses of count() but tests fail.

This is also starting to hit the limits of using PHPUnit 4, I'd recommend dropping some of the older versions of PHP from the support matrix.

Rename Object to JsonLDObject
Don't assume its an array
@whikloj

This comment has been minimized.

Contributor

whikloj commented Apr 10, 2018

Of interest to @acoburn and @DiegoPino

@osma

This comment has been minimized.

osma commented Apr 12, 2018

+1. We are also struggling with 7.2 compatibility, merging this into an official release available via Composer would help a lot!

@whikloj

This comment has been minimized.

Contributor

whikloj commented Apr 19, 2018

@lanthaler If there is something I can do to make this easier, please let me know. I realize that you may not want to use JsonLDObject and I have no issue changing that if it helps you.

@rdewaele

This comment has been minimized.

rdewaele commented Jul 19, 2018

We also encountered this problem while using api-platform with this plugin to generate the PHP classes.
@lanthaler Could you share an update on reviewing and/or merging this PR?

@Mehrdad-Dadkhah

This comment has been minimized.

Mehrdad-Dadkhah commented Aug 2, 2018

same problem, please merge this PR

@whikloj

This comment has been minimized.

Contributor

whikloj commented Oct 25, 2018

Hi @lanthaler,

6 months in and there has been no response on this PR. Is there something I can do to help move this forward?

@dougblackjr

This is an excellent way to address the php7.2 issues mentioned.

@DiegoPino

This comment has been minimized.

DiegoPino commented Nov 13, 2018

@lanthaler I know you are very busy dealing with JSON-LD 1.1 specs and your day job too, but it would mean a great deal to us == highly appreciated == if you could review/merge this pull. Many of us are using this package (great one) on current and future larger community-driven Open Source projects and having PHP 7.2 support is crucial. Thanks!

@lanthaler lanthaler merged commit fee3546 into lanthaler:master Nov 18, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@osma

This comment has been minimized.

osma commented Nov 18, 2018

Yay, finally! Thanks a lot @lanthaler!

@DiegoPino

This comment has been minimized.

DiegoPino commented Nov 18, 2018

@lanthaler++ thanks a lot

@whikloj whikloj deleted the whikloj:fix-php-72 branch Nov 19, 2018

@whikloj

This comment has been minimized.

Contributor

whikloj commented Nov 19, 2018

Thank you @lanthaler, I would have been happy to make the desired changes but either way I am glad to get this working. Cheers.

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