-
Notifications
You must be signed in to change notification settings - Fork 27
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
Update SDK to support Illuminate/support 7.x branch (and Laravel 7.x) #217
Comments
Having this issue as well. |
My current solution is terrible and ugly as ... but it works for the time being. Implement the above changes to the After deploy of your code to the server, overwrite the Collection.php file in the vendor folder
|
Saw that part of this is fixed by 648c7db. Except that Illuminati v7.5.2 is moving further and has changed the declaration of the
Maybe make two tags when it gets merged into master to let people lock down a combination of myparcel+illuminate in their composer.json? |
This is resolved in version 4.0.0. @appelflap @JBtje @Cannonb4ll Can you test whether it works now? |
Tried to update, but the 4.x isn't on packagist yet @reindert-vetter |
Hi @appelflap , |
Yes, see it now too. Page says: Looks good! Stable with: illuminate/http (v5.6.0 => v7.10.3) |
We are still having this issue on laravel/framework (v7.10.3) with myparcelnl/sdk (v4.0.0):
|
@RickJeroen, do you also have illuminate/support v7.10.3? Because MyParcelNL\Sdk\src\Support\Collection (indirectly) implements the EnumerateValuesTrait from that package andhas function definition |
@appelflap @RickJeroen @JBtje Do you like or dislike the fact that we extend the standard laravel class? It does force us to stay up to date with the latest changes in Laravel collection |
No problems with it being extended, but the dependency is not in the project composer.json. So version compatibility is somewhat unclear. |
I agree |
agree as well |
Just tested #230, it works for laravel 7.4-7.11 (latest). Though, as stated by Appelflap, adding dependency requirement in composer.json would be nice. |
Still having the same error, have not tried #230 but would be nice if that got merged.
|
Given a clean Laravel 7.x project, adding the myparcel SDK will result in the errors below.
Cause of the problem: Laravel 7.x is using the Illuminate/support 7.x branch, whilst MyParcel is using a version of branch 5.6
Quick and dirty solution:
https://github.com/myparcelnl/sdk/blob/master/src/Support/Collection.php#L1238
https://github.com/myparcelnl/sdk/blob/master/src/Support/Collection.php#L1445
For me, this solves the problem and the SDK is still working... Better would be to implement the new Collection.php
The text was updated successfully, but these errors were encountered: