Allow brick math 0.15 and 0.16#633
Conversation
0.15 removed the 'old' rounding mode constants, so this includes a BC layer in the `BrickMathCalculator`
|
After creating PR #634 for the By the way, is there an ETA for the v5 release? |
|
|
||
| $fallbackBuilder->build($codec, $bytes); | ||
| } | ||
| // public function testBuildThrowsExceptionAfterAllConfiguredBuildersHaveErrored(): void |
There was a problem hiding this comment.
Why are these tests commented out?
There was a problem hiding this comment.
My bad, i think i commented them out to debug something. It should be fine to uncomment them again
|
It looks like brick/math broke a lot of BC with earlier releases and removed or moved a lot of things. That's why there are so many PHPStan errors. Is that something you're able to help clean up? |
No problem. My suggestion would be to 'simply' add a bunch of I think the most maintainable solution for this library is to get this MR (or another with a BC layer) merged and tagged. This would give users of the package an easier upgrade path, as this can be upgraded first, and then brick/math later. But it would introduce the least strain on maintainability. Is that a solution that works for you? |
These should be safe to remove when the BC layer is removed again
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.x #633 +/- ##
============================================
- Coverage 95.12% 94.48% -0.64%
- Complexity 575 578 +3
============================================
Files 70 70
Lines 1640 1668 +28
============================================
+ Hits 1560 1576 +16
- Misses 80 92 +12
|
|
I have added the ignores, and made an example of dropping the BC layer again and requiring the newer brick math versions: BackEndTea#1 |
| */ | ||
| final class BrickMathCalculator implements CalculatorInterface | ||
| { | ||
| private const ROUNDING_MODE_MAP = [ |
There was a problem hiding this comment.
private const ROUNDING_MODE_MAP = [
RoundingMode::UNNECESSARY => BrickMathRounding::Unnecessary ?? BrickMathRounding::UNNECESSARY,
...Alternatively and a matter of taste, but this could also work instead of the static $roundingModeMap.
With this approach, you don't need defined(). The whole compat layer would be these 10 lines.
|
With #638 merged I guess this is now obsolete? |
5 participants
0.15 removed the 'old' rounding mode constants, so this includes a BC layer in the
BrickMathCalculatorDescription
This MR allows installation of brick math versions 0.15 and 0.16.
0.14 changed the rounding mode to an enum, but kept backwards compatability. in 0.15 this BC layer was removed.
To support older brick/math versions (and older PHP versions), we introduce a BC layer in the
BrickMathCalculatorclass.Motivation and context
How has this been tested?
I have ran the phpunit tests with brick/math 0.14, 0.15 and 0.16.
Types of changes
PR checklist
PHPUnit test are working for this MR, but there are some PHPStan failures. I'm not sure how to move forward with those.