feat: Support brick/math v0.12 #526
Conversation
Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
…be ignored) Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
Signed-off-by: Natsuki Ikeguchi <me@s6n.jp>
| include: | ||
| # Keep the locked version by default | ||
| - dependency-versions: "locked" | ||
| # For PHP 8.0, installing with --prefer-highest to use brick/math v0.11 | ||
| - php-version: "8.0" | ||
| dependency-versions: "highest" |
There was a problem hiding this comment.
Since brick/math is now locked at v0.12.1 (which requires PHP >= 8.1) in composer.lock, CI can not install it on PHP 8.0. Using --prefer-highest to avoid the version and use v0.11. Let me know if there is much better solution.
There was a problem hiding this comment.
maybe drop support for PHP 8.0
There was a problem hiding this comment.
I agree, PHP 8.0 is now EOL so we can remove the support.
Let @ramsey decide anyway.
| @@ -11,7 +11,7 @@ | |||
| "require": { | |||
| "php": "^8.0", | |||
| "ext-json": "*", | |||
| "brick/math": "^0.8.8 || ^0.9 || ^0.10 || ^0.11", | |||
| "brick/math": "^0.8.8 || ^0.9 || ^0.10 || ^0.11 || ^0.12", | |||
There was a problem hiding this comment.
FYI: I tried installing brick/math with --prefer-lowest and run a static analysis, but it failed since BigInteger::toBytes does not exist in v0.8.8. This is out of scope for this PR, so I ignored.
| @@ -4,7 +4,8 @@ | |||
| xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" | |||
| errorLevel="1" | |||
| cacheDirectory="./build/cache/psalm" | |||
| errorBaseline="psalm-baseline.xml"> | |||
| errorBaseline="psalm-baseline.xml" | |||
| phpVersion="8.1"> | |||
There was a problem hiding this comment.
Psalm infers the target PHP version from composer.json by default. Enum classes add in brick/math v0.12 are ignored in PHP 8.0, so fixed to PHP 8.1 as the target to avoid it.
| { | ||
| return self::ROUNDING_MODE_MAP[$roundingMode] ?? 0; | ||
| return self::ROUNDING_MODE_MAP[$roundingMode] ?? BrickMathRounding::UNNECESSARY; |
There was a problem hiding this comment.
BrickMathRounding::* is a enum case in v0.12 and is a const value in v0.11.
|
This issue blocks our upgrade because we use another package requiring brick/math 0.12+. Is there anything I can do to help to make this merged? |
|
Sorry for the delay. I've been dealing with some rough issues in my personal life, most notably, I'm unemployed at the moment, so every waking hour is spent job-searching, and I haven't had time for open source contributions. I will try to revisit this and merge and release it as soon as I can, but it might still be a while. Thank you for your patience! |
|
@ramsey any updates on this issue? |
|
Hi @ramsey - hope all is well with you. Is there anything we can do to help get this one over the line? It's a showstopper for multiple projects for me that need to be using Brick/Money which requires Brick/Math 0.12+. Thanks! |
|
@ramsey Hope you are doing well. Hoping to merge this one soon. If there is anything we can do, let us know. Thank you. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.x #526 +/- ##
=========================================
Coverage 95.64% 95.64%
Complexity 583 583
=========================================
Files 70 70
Lines 1560 1560
=========================================
Hits 1492 1492
Misses 68 68
|
|
Thank you for contributing! 🎉 |
Closes #525
Description
This pull request adds support for brick/math v0.12.
Motivation and context
brick/math v0.12 has been released.
ramsey/uuid requires < 0.12 for now, so we can not upgrade brick/math until ramsey/uuid supports it.
How has this been tested?
Static Analysis + Unit Tests
Types of changes
PR checklist