Skip to content

Support brick/math 0.18 and 0.19 - #6

Open
robertvansteen wants to merge 2 commits into
mainfrom
brick-math
Open

Support brick/math 0.18 and 0.19#6
robertvansteen wants to merge 2 commits into
mainfrom
brick-math

Conversation

@robertvansteen

Copy link
Copy Markdown
Contributor

Widens brick/math from ^0.12 || ^0.13 to ^0.18 || ^0.19, which is what a consumer on a current framework needs.

The breaking part: brick no longer accepts a float in arithmetic or comparison, because a binary float carries no exact decimal value to compare an interval bound against — the literal 0.1 is not one tenth. So float is dropped from the four endpoint comparison signatures:

-    public function isLessThan(BigNumber|int|float $value): bool
+    public function isLessThan(BigNumber|int $value): bool

Callers pass a BigNumber, an int, or a numeric string. This is why the release is 2.0.0.

Two fixes fall out of the newer toolchain:

  • leftEndpoint / rightEndpoint always exist once the pattern matches, so the ?? null on each was dead — unmatched groups arrive as '', which the checks below already treat as an open bound.
  • Every inequality case in equalityCases differed only in notation, so nothing pinned the endpoint comparison and a mutant weakening its conjunction to a disjunction survived. Two cases now share one endpoint and differ in the other.

PHPUnit is capped below 12.5.12, which rejects the -d --min-coverage form phpunit-coverage-tools documents for its threshold.

Verified: PHPStan clean, 88 tests, 100% coverage, MSI 100%.

brick/math no longer accepts floats in arithmetic or comparison, so the float
member is dropped from the four endpoint comparison signatures. A binary float
carries no exact decimal value to compare an interval bound against; callers
pass BigNumber, int or a numeric string.

Two fixes fall out of the newer toolchain:

- The named groups leftEndpoint and rightEndpoint always exist once the pattern
  matches, so the ?? null on each was dead. Unmatched groups arrive as '' and
  the checks below already treat that as an open bound.
- Every inequality case in equalityCases differed only in notation, so nothing
  pinned the endpoint comparison and a mutant weakening its conjunction to a
  disjunction survived. Two cases now share one endpoint and differ in the
  other.

PHPUnit is capped because 12.5.12 rejects the "-d --min-coverage" form that
phpunit-coverage-tools documents for its coverage threshold.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 90b1f7bb06

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread composer.lock Outdated
"symfony/polyfill-intl-grapheme": "~1.0",
"symfony/polyfill-intl-normalizer": "~1.0",
"symfony/polyfill-mbstring": "~1.0"
"php": ">=8.4.1",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep the lock file installable on PHP 8.3

With the root package still advertising PHP ^8.3, a full composer install on PHP 8.3 now fails because the lock pins symfony/string to v8.1.2, whose own requirement is >=8.4.1; I confirmed with PHPENV_VERSION=8.3snapshot composer install --dry-run, which stops before installing the dev toolchain. Please lock a 7.x-compatible symfony/string or raise the package's minimum PHP so contributors/CI on the supported minimum can install dependencies.

Useful? React with 👍 / 👎.

Comment thread composer.json Outdated
"php": "^8.3",
"webmozart/assert": "^1.11",
"brick/math": "^0.12.0 || ^0.13.0"
"brick/math": "^0.19.0"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Allow brick/math 0.18 in the constraint

For consumers whose framework is already locked to brick/math 0.18.x, this dependency still makes v2 impossible to install because ^0.19.0 resolves only within the 0.19 line. If this release is meant to support both current 0.18 and 0.19 users, widen the constraint to include ^0.18.0 as well; otherwise the advertised compatibility gap remains.

Useful? React with 👍 / 👎.

Two corrections to this branch.

The constraint said ^0.19.0, which excludes consumers whose framework caps
brick/math at 0.18 — the case this release exists to serve. It now accepts
^0.18 || ^0.19, matching what the title always claimed.

The lock had drifted to symfony/string v8.1.2, which requires PHP >= 8.4.1,
while this package advertises ^8.3 — so `composer install` on the advertised
minimum failed before reaching the dev toolchain. Resolution is now pinned to
the minimum supported PHP through config.platform, as axiom does, so the
committed lock is installable on every version the package claims to support
and cannot silently drift again on a maintainer's newer runtime.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant