-
Notifications
You must be signed in to change notification settings - Fork 56
Laravel 5.4 Compatibility #27
Comments
@azcoppen Thanks! Will try to get to this tonight and will push a 5.4 branch if it is alright with you? I haven't had time to update my projects |
@azcoppen for the warning, could it be because you pull the cloudinary_php library in your composer.json? It will be included by default when requiring cloudder, so maybe that can help! |
@jrm2k6 That would be fantastic chap, thank you! I forked it into a private repo. Can do a PR if you like. The issue is Laravel 5.4 removes share() in the register() method of the service provider. You just need to change share() to singleton(). Edit: this might require more, as i'm getting "illegal offset type" with PHP7.1. Will need your update - will wait for it instead of using a fork as it's causing errors atm. Edit: Also, am not requiring Cloudinary API in composer. |
Also, another error i'm getting here:
|
Thanks for the hint, it should make my job easier. Will keep you up to date.
…On Tue, Jan 31, 2017 at 10:08 AM azcoppen ***@***.***> wrote:
Also, another error i'm getting here:
Warning: Ambiguous class resolution, "Cloudinary\Akamai" was found in both "vendor/cloudinary/cloudinary_php/src/Akamai.php" and "vendor/cloudinary/cloudinary_php/src/Cloudinary/Akamai.php", the first will be used.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABCaY_4JDyYDMLFwtGXR0VJPgO9BVLb5ks5rX3iUgaJpZM4Ly7T9>
.
|
@jrm2k6 It's an incredibly useful package - thanks for your work. Only 1 more thing - any chance you could change the Facade name to something more uniform than Cloudder? Would be great to just use Cloudinary::upload()! |
I am not sure I can change the name as I am not affiliated with Cloudinary in any way. Not sure I can use that name. |
@azcoppen @nicodebin Just pushed a new branch, could you guys check and make sure it works? Works fine on my local. |
@nicodebin Just merged it, thanks for your PR. I will double check that maybe on my lunch break, and will merge/release if it works fine. It looks like it is backwards compatible so all good! Thanks for your patience. I still have most of my side projects running on Laravel 5.2. 🚒 |
@jrm2k6 cool thanks! Yeah, me too.. most of my projects are running in 5.2 too, and a very huge one in 4.2 😱 |
@jrm2k6 Testing later tonight - thanks so much for the speed you responded to the issue with! |
@azcoppen let me know when it is confirmed, will try to release asap. |
@jrm2k6 Yep, looks great!
|
Alright will release soon
…On Fri, Feb 3, 2017, 9:52 AM azcoppen ***@***.***> wrote:
Closed #27 <#27>.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#27 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABCaYzoaCKrPICu7kD1M3WUxtXVkC3Tlks5rY2l1gaJpZM4Ly7T9>
.
|
2 errors on update:
The text was updated successfully, but these errors were encountered: