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.

Sign up for GitHub

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

Jump to bottom

feat: Support brick/math v0.12 #526

Merged
merged 5 commits into from Apr 27, 2024
+39 −17
Merged

feat: Support brick/math v0.12 #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7316a40
build(deps): Allow brick/math v0.12
siketyan Nov 30, 2023
2694b0e
fix: Use BrickMathRounding::UNNECESSARY for the fallback
siketyan Nov 30, 2023
651f230
build: Run Psalm for PHP 8.1 as the target (to avoid enum classes to …
siketyan Nov 30, 2023
d02bb50
ci: Use brick/math v0.11 on PHP 8.0
siketyan Nov 30, 2023
163767b
Merge branch '4.x' into feat/brick-math-0.12
ramsey Apr 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion .github/workflows/continuous-integration.yml
View file
Open in desktop
Expand Up @@ -79,6 +79,12 @@ jobs:
- "8.1"
- "8.2"
- "8.3"
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"
Comment on lines +82 to +87
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe drop support for PHP 8.0

https://www.php.net/supported-versions.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, PHP 8.0 is now EOL so we can remove the support.
Let @ramsey decide anyway.


steps:
- name: "Checkout repository"
Expand All @@ -99,6 +105,8 @@ jobs:

- name: "Install dependencies (Composer)"
uses: "ramsey/composer-install@v3"
with:
dependency-versions: "${{ matrix.dependency-versions }}"

- name: "Run PHPBench"
run: "composer phpbench -- --ansi"
Expand Down Expand Up @@ -151,6 +159,12 @@ jobs:
operating-system:
- "ubuntu-latest"
- "windows-latest"
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"

steps:
- name: "Configure Git (for Windows)"
Expand Down Expand Up @@ -179,7 +193,7 @@ jobs:
- name: "Install dependencies (Composer)"
uses: "ramsey/composer-install@v3"
with:
composer-options: "${{ matrix.composer-options }}"
dependency-versions: "${{ matrix.dependency-versions }}"

- name: "Run unit tests (PHPUnit)"
run: "./vendor/bin/phpunit --verbose --colors=always --no-coverage"
2 changes: 1 addition & 1 deletion composer.json
View file
Open in desktop
Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

"ramsey/collection": "^1.2 || ^2.0"
},
"require-dev": {
Expand Down
29 changes: 17 additions & 12 deletions composer.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion psalm.xml
View file
Open in desktop
Expand Up @@ -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">
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


<projectFiles>
<directory name="./src" />
Expand Down
6 changes: 4 additions & 2 deletions src/Math/BrickMathCalculator.php
View file
Open in desktop
Expand Up @@ -136,9 +136,11 @@ public function toInteger(Hexadecimal $value): IntegerObject

/**
* Maps ramsey/uuid rounding modes to those used by brick/math
*
* @return BrickMathRounding::*
*/
private function getBrickRoundingMode(int $roundingMode): int
private function getBrickRoundingMode(int $roundingMode)
{
return self::ROUNDING_MODE_MAP[$roundingMode] ?? 0;
return self::ROUNDING_MODE_MAP[$roundingMode] ?? BrickMathRounding::UNNECESSARY;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BrickMathRounding::* is a enum case in v0.12 and is a const value in v0.11.

}
}