Skip to content

Modernize for 4.0.0: PHP 8.1+, full typing, PHPUnit 10-12, GitHub Actions - #26

Open
serima wants to merge 1 commit into
masterfrom
feature/modernize-v4
Open

Modernize for 4.0.0: PHP 8.1+, full typing, PHPUnit 10-12, GitHub Actions#26
serima wants to merge 1 commit into
masterfrom
feature/modernize-v4

Conversation

@serima

@serima serima commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

Modernizes the library as the 4.0.0 release while keeping it safe for existing users:

  • PHP ^8.1 minimum (was >=7.2), with full parameter, return and property type declarations throughout.
  • Fixes a regression in the unreleased binary-search lookup (use binary search for lookup #24): lookupList() could return fewer targets than requested when adjacent ring positions belonged to the same target (with the default 64 replicas this affects roughly 1 in 10 keys for lookupList($key, 2)). Duplicates are now skipped, restoring the v3.0.0 semantics. Regression tests included, verified to fail against the previous implementation.
  • Fixes the PHP 8.4 deprecation for implicitly nullable constructor parameters.
  • HasherInterface::hash() is now declared as hash(string $string): int|float (BC break for custom hashers, documented in the CHANGELOG).
  • Travis CI + Coveralls replaced by GitHub Actions: tests on PHP 8.1/8.2/8.3/8.4/8.5, plus PHPCS and PHPStan jobs.
  • PSR-2 → PSR-12, PHPStan at level 9, PHPUnit 10/11/12 with attributes.
  • Drops the committed composer.lock, adds composer scripts (test, benchmark, cs, cs:fix, stan, check), refreshes README/CHANGELOG/ROADMAP.

Compatibility

Hash values and position derivation are unchanged. Verified by running identical workloads (CRC32 and MD5 hashers, 2000 keys, weighted targets, target removal) against the v3.0.0 implementation: lookup results are byte-for-byte identical, so upgrading does not remap keys.

Existing 2.x/3.x users are unaffected per SemVer; documented BC breaks only affect custom HasherInterface implementations (must add the return type) and strict_types callers passing non-string targets.

Verification

  • PHP 8.5.7 / PHPUnit 12.5: 26 tests green, PHPCS clean, PHPStan level 9 clean, benchmarks run.
  • PHP 8.1.34 / PHPUnit 10.5: same, all green.

🤖 Generated with Claude Code

…ions

- PHP ^8.1 minimum (was >=7.2); full parameter, return and property
  type declarations throughout
- Fix lookupList() returning fewer targets than requested when adjacent
  ring positions belong to the same target (regression in the unreleased
  binary-search lookup, #24), with regression tests
- Fix implicitly nullable constructor parameters deprecated in PHP 8.4
- Fix BenchmarkTest::median() element selection and TypeError under
  strict_types
- HasherInterface::hash() now declared as hash(string $string): int|float
- Replace Travis CI and Coveralls with GitHub Actions (PHP 8.1-8.5,
  PHPCS, PHPStan)
- Upgrade PSR-2 to PSR-12; add PHPStan at level 9
- PHPUnit 10/11/12 with attributes instead of annotations
- Drop committed composer.lock; add composer scripts (test, benchmark,
  cs, cs:fix, stan, check)

Hash values and position derivation are unchanged: upgrading does not
remap keys to different targets (verified against v3.0.0 with CRC32 and
MD5 hashers over 2000 keys, including weighted targets and removal).

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

serima commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

@pda @dmnc Before I merge this, I'd like to get a green light from you since it's a major release (4.0.0) — no rush, and happy to adjust anything.

A quick summary of what this does and why I believe it's safe:

  • No key remapping. Hash values and position derivation are untouched. I verified lookup results byte-for-byte against v3.0.0 (CRC32 and MD5 hashers, 2000 keys, weighted targets, removal scenarios) — they are identical, so existing rings are unaffected by the upgrade.
  • Fixes a regression that is currently sitting unreleased on master (use binary search for lookup #24, the binary-search lookup): lookupList() could return fewer targets than requested when adjacent ring positions belong to the same target. With the default 64 replicas this affects roughly 1 in 10 keys for lookupList($key, 2). This PR restores the v3.0.0 semantics with regression tests, so I'd like to get this merged before anything else ships from master.
  • BC breaks are intentional and documented in the CHANGELOG: PHP ^8.1 floor, typed HasherInterface::hash(string $string): int|float (custom hashers need a compatible return type), and typed parameters on Flexihash methods. 2.x/3.x users are unaffected per SemVer.
  • Tooling refresh: Travis CI/Coveralls → GitHub Actions (PHP 8.1–8.5, all green on this PR), PSR-2 → PSR-12, PHPStan level 9, PHPUnit 10/11/12.

My plan after approval: merge, then create the v4.0.0 GitHub release (Packagist picks it up automatically).

If either of you would rather keep a lower PHP floor, split the release differently, or take over the release itself, just say the word.

@serima
serima requested review from dmnc and pda July 5, 2026 16:01
@dmnc

dmnc commented Jul 15, 2026

Copy link
Copy Markdown
Collaborator

@serima this all looks very sensible to me and I think the BC interface changes are absolutely fine given it is a breaking version and you have kept the hashing consistent.

Unfortunately I don't use flexihash in any projects at the moment as my big historical use case was sharding assets across CDN domains which is no longer necessary with HTTP/2. This means I can't really test it in an application.

The only additional thing I might recommend is adding https://packagist.org/packages/rector/rector and a corresponding config file so that this sort of process can be automated in future.

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.

2 participants