-
Notifications
You must be signed in to change notification settings - Fork 91
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
Add a PHP 5.4 Requirement #103
Comments
Introduced in #91 - can the constant be removed without breaking the functionality? |
There are php functions that have been developed to workaround this particular issue |
It's possible that they would be overkill in this case. If the JSON doesn't need to be pretty printed in the first place, those functions are superfluous overhead. |
Well, pretty printed json makes it readable. If you have a big nested json it can be very hard to read sometimes. |
So this is output for a human, not for other parts of the application? |
it's only used in the admin view of running and failed jobs - not in the actual application, no however, we should probably fix this issue, but right now i just don't have the time ... PR on the matter will be taken care of tho. |
I will remove the constant that makes it pretty printed for now and open a PR so it can work on PHP 5.3, then maybe we can discuss other implementations. |
Instead of removing it, It is actually possible to achieve JSON pretty print with backwards graceful degradation.
More reading here http://stackoverflow.com/a/22208945/515268 |
It seems safer to
|
Hi,
I am on PHP 5.3 and this was working until I actually loaded a worker and job. It seems that it depends on JSON_PRETTY_PRINT which was not introduced until PHP 5.4.0. I recommend either making it use a helper function for older php versions or making php 5.4 a requirement in the composer.json
The text was updated successfully, but these errors were encountered: