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

[13.x] Allow latest moneyphp version #1280

Merged
merged 1 commit into from
Nov 19, 2021

Conversation

mmachatschek
Copy link
Contributor

This is a breaking change for some users, as moneyphp version 4 requires the bcmath extension

@driesvints
Copy link
Member

Hmm, technically we could add this to the current stable release. Composer will handle the extension checking. @taylorotwell what do you think?

@driesvints
Copy link
Member

Almost every Forge site should have bcmath installed and did an ask around for people using Cashier/Spark Stripe. All had bcmath installed. So I think we can move this to 13.x.

@mmachatschek mmachatschek changed the base branch from master to 13.x November 19, 2021 11:46
@mmachatschek
Copy link
Contributor Author

I rebased the PR to the 13.x branch

@mmachatschek mmachatschek changed the title [14.x] Allow latest moneyphp version [13.x] Allow latest moneyphp version Nov 19, 2021
@taylorotwell
Copy link
Member

I dunno - is it not a breaking change to now require a new PHP extension? I don't see how that is not a breaking change.

@driesvints
Copy link
Member

driesvints commented Nov 19, 2021

@taylorotwell adding new dependencies without breaking the public API is allowed under SemVer: https://semver.org/#what-should-i-do-if-i-update-my-own-dependencies-without-changing-the-public-api

Composer will prevent Cashier from being installed onto systems which don't have bcmath bundled in PHP. I believe the impact of this will be very minimal. The only thing people need to do is compile bcmath with PHP onto their systems if they want to composer update to the latest Cashier (which is a very small % of people).

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.

3 participants