-
Notifications
You must be signed in to change notification settings - Fork 682
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
10.3.0 introducing "breaking changes" #794
Comments
Which things broke for you specifically? |
We used the Slightly unrelated, but this method could be perfectly implemented with a LazyCollection and I plan to make a pull request about this against master (since this requires L6.0 features). Any opinion about this? |
Hmm, I can see how that could be breaking to you yeah. But the thing is what you have to know is that when you directly use features of an SDK outside the public api of a package it's best that you also add the constraint to your Btw, the problem you're describing is actually documented under SemVer here: https://semver.org/#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api If you depend on specific functionality of a library's dependencies then you need to add that library to the list of your own dependencies.
Ah yeah that's a good idea actually. You can definitely send in a pr to master if you want. Maybe create a different issue for that. |
Yeah, that's why I said it should be marked as internal (with |
@driesvints I don't know if this counts as a breaking change or not so I'm not opening a new issue but all exceptions were replaced in v7 of stripe, so if anybody was handling those that could be a problem. Reference: stripe/stripe-php#709 |
@netpok well, as I said: if you were relying on exceptions from the stripe sdk then you would need to add the sdk to your own composer.json. |
@taylorotwell As you said in #796 you don't consider asStripe... internal methods please have a look at this issue. In short: With the update 10.3.0 the underlying stripe-php library was updated to v7, which largely changed the interface objects returned by the asStripe... methods. Furthermore it changed all the exceptions thrown by the library and passed through by Cashier, so if one checked for Stripe\Error\Api to detect a network error it fails now. If this kind of change can happen in the future I'm happy to write a small section to the docs about accessing underlying objects (I don't think exceptions will change in the future). |
@netpok a section in the docs could definitely be helpful. Feel free to send in a pr. Thanks :) |
Description:
With Cashier 10.3.0 the underlying Stripe-PHP SDK was updated to v7, while this api is not created by this project, but it is directly exposed via the
asStripe...
methods which are not marked as internal methods. Following an update we noticed that many of our calls failed because of this.I think either these kind of changes should be considered as breaking changes and implemented in new major versions or these method should be marked as internal.
The text was updated successfully, but these errors were encountered: