Fix division without bcmath and using the precision parameter #19
Conversation
Maybe it would have been easier to submit different pull requests for each item 😛 |
No time, do you have time? The fix is a one line, and couldn't wait 3 years before being merge. Please rebase, review and merge fast! |
| */ | ||
| public function testItThrowsExceptionIfRoundingModeIsInvalid() | ||
| { | ||
| $this->expectException(InvalidArgumentException::class); |
There was a problem hiding this comment.
Agreed. I prefer assertions rather than phpDoc magic annotation
There was a problem hiding this comment.
The magic annotation has been deprecated and no longer work in the next release, that's the reason ^^
There was a problem hiding this comment.
I prefer the latest trends always
| excludes_analyse: | ||
| - vendor/ | ||
| reportUnmatchedIgnoredErrors: false | ||
| level: max |
| $result = $this->integerDivision($a, $b, max($precision, $aPrecision)); | ||
|
|
||
| return $result; | ||
| return $this->integerDivision($a, $b, $precision); |
There was a problem hiding this comment.
In the current implementation
- if
$aPrecision > $precisionit uses$aPrecisionalthough$precisionhave been requested by user - if
$bPrecision > $aPrecisionit uses$aPrecision
I agree your code seems more valid.
Do I even know php? |
Who need to know something someone already know better than you need to know! 👀 |
|
I approve the PR and CI is green. We could merge or maybe ask for another review from @PrestaShop/prestashop-maintainers |
|
Thanks @PierreRambaud |
The actual fix about precision: 5d39d4e#diff-25c0e6754f8433ec8448cb2b71ba8d0a66b6574dcb8ed25e27a87eaf28c58996R115