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

Add some backward compatibility for PHP 5.3 and 5.4 versions #40

Merged
merged 3 commits into from
Aug 28, 2019

Conversation

Darkkin
Copy link
Contributor

@Darkkin Darkkin commented Aug 22, 2019

No description provided.

src/Client.php Outdated
* Function added for backward compatibility with PHP version < 5.5
*/

private function json_last_error_msg() {
Copy link
Member

@FZambia FZambia Aug 22, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand right that PHP >5.4 will use this function too instead of native one?

@Darkkin
Copy link
Contributor Author

Darkkin commented Aug 22, 2019

Yes, you're right. Do you think it's better to create a wrapper function with function_exists() check?

@FZambia
Copy link
Member

FZambia commented Aug 22, 2019

@Darkkin yes, just to inherit all changes from this function in new PHP versions and fallback to this one only on old PHP versions.

@Darkkin
Copy link
Contributor Author

Darkkin commented Aug 23, 2019

Request updated. Please review added wrapper.

src/Client.php Outdated

public function _json_last_error_msg() {
if (function_exists('json_last_error_msg')) {
return json_last_error_msg(json_last_error());
Copy link
Member

@FZambia FZambia Aug 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at https://www.php.net/manual/en/function.json-last-error-msg.php json_last_error_msg definition is:

json_last_error_msg ( void ) : string

Here you are calling it with argument json_last_error() - can you explain why?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's my mistake. Fixed.

@FZambia FZambia merged commit 12bdaf2 into centrifugal:master Aug 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants