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

Added openswoole extension check compatibility #506

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

nahid
Copy link
Contributor

@nahid nahid commented Nov 4, 2021

Swoole now became openswoole, from version 4.7.2 swoole extension changed as openswoole. For this reason, this package can not detect the extension and shows the message Can't detect Swoole extension installed. This commit fixed the issue.

Swoole now became `openswoole`, from version 4.7.2 swoole extension changed as openswoole. For this reason, this package can not detect the extension and shows the message *Can't detect Swoole extension installed.* This commit fixed the issue.
@@ -370,7 +370,7 @@ protected function checkEnvironment()
exit(1);
}

if (! extension_loaded('swoole')) {
if (! extension_loaded('swoole') && ! extension_loaded('openswoole')) {
Copy link
Member

Choose a reason for hiding this comment

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

shouldn't this be an or statement?

...
if (! extension_loaded('swoole') or ! extension_loaded('openswoole')) {

...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Arkanius thanks for your reply. Nope it will be and operator. bcoz Sometimes one of the two may be available but it does not mean that one of them missing means others is not available. when both of them are missing then we should show the message.

@Arkanius
Copy link
Member

Thanks for your contribution! Sorry, I was on vacation and couldn't see this before.

I've just one doubt, please check my review

@nahid
Copy link
Contributor Author

nahid commented Nov 23, 2021

@Arkanius please check the review

@Arkanius
Copy link
Member

Arkanius commented Dec 2, 2021

Got it! You're right! we'll apply your PR!

Thanks for contributing!

@Arkanius Arkanius merged commit e00771a into swooletw:master Dec 2, 2021
@ErriourMe
Copy link

When it will be released? (:

@madhurbhaiya
Copy link

Hi @Arkanius

Please release this minor change. Otherwise, we need to Extend the Command and Service Provider in our code, just to get OpenSwoole working with Laravel

@Arkanius
Copy link
Member

Arkanius commented Jan 20, 2022

@madhurbhaiya @ErriourRU sorry by my late response here guys, I had some problems and was really busy, I'm sorry again =/

Well, it's merged, but please, test it out and check if your apps is running well. I'll be wating your answer

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.

None yet

4 participants