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

Incompatible with PHP 7.3 #19

Open
VarunAgw opened this issue Apr 10, 2021 · 12 comments
Open

Incompatible with PHP 7.3 #19

VarunAgw opened this issue Apr 10, 2021 · 12 comments

Comments

@VarunAgw
Copy link

VarunAgw commented Apr 10, 2021

Please test it thoroughly before publishing it to production.

https://www.php.net/manual/en/language.types.declarations.php#language.types.declarations.mixed

The feature is introduced in PHP 8.0, yet it's used widely in the code.

Example error: ( ! ) Fatal error: Uncaught TypeError: Return value of KiteConnect\KiteConnect::getPositions() must be an instance of KiteConnect\mixed, instance of stdClass returned in D:\www\trading\vendor\zerodha\phpkiteconnect\src\KiteConnect.php on line 648

There might be more issues with PHP 7.3. Please test it thoroughly with PHP 7.3.

@vividvilla
Copy link
Member

@ranjanrak Can you take a look at this?

@ranjanrak
Copy link
Member

@VarunAgw
The issue seems to be at Composer file, as it has captured the minimum required PHP version as 7.3. Checking on this composer file issue.
We have thoroughly tested this on our local setup including running the added unit tests. Our local setup has PHP 8.0.3. Mostly, we will update the package with the minimum requirement of PHP >=8. For the older PHP version, users can always refer to our older version, as stated in README.

@VarunAgw
Copy link
Author

thissssssss

@VarunAgw
Copy link
Author

Besides, it's terrible to force people to use PHP 8.0 which was released just 4 months ago.

You have to understand that people run multiple projects other than Zerodha PHP SDK on their development/production machine.

PHP 8 has breaking changes. By upgrading Zerodha SDK, I am not going to break all my other projects whose dependencies are not compatible with PHP 8 yet. You have to give the entire PHP eco-system time to upgrade before forcing such latest standards.

@ranjanrak
Copy link
Member

@VarunAgw

PHP 8 has breaking changes. By upgrading Zerodha SDK, I am not going to break all my other projects whose dependencies are not compatible with PHP 8 yet. You have to give the entire PHP eco-system time to upgrade before forcing such latest standards.

Thanks for your input. We are re-evaluating our PHP version compatibility for the recent release.

@VishalKumarSahu
Copy link

Return type (used in getPositions and getMargins) mixed is not implemented in php7.3 yet. It's in php8.

@sathyamoorthi
Copy link

getLTP function also is not working in > 7.3. Can you please fix it?

@ranjanrak
Copy link
Member

You need to upgrade to PHP > 8.0 for now.

@VarunAgw
Copy link
Author

KiteConnect.zip

Here is the PHP 7.3 fixed I made 6 months ago. Feel free to compare it with master before using it.

@sathyamoorthi
Copy link

sathyamoorthi commented Dec 16, 2021

@ranjanrak But in "Requirements" section you mentioned 7.3 or greater !!? At least you should update README.

Thanks @VarunAgw

@VarunAgw
Copy link
Author

@sathyamoorthi Don't expect too much here. Typical Indian companies attitude.

@vishalmote
Copy link

KiteConnect.zip

Here is the PHP 7.3 fixed I made 6 months ago. Feel free to compare it with master before using it.

Not compatible, still same exception throwing,

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 a pull request may close this issue.

6 participants