Modernise: native types, multi-engine CI, static analysis, Testbench#2
Merged
Conversation
`(array)$node` casts an object to its properties, not its used traits, so isNode() returned false for genuine nodes. Use class_uses_recursive() with strict comparison instead. Add a regression test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adopt the Laravel preset via pint.json and apply it across src/ and tests/. Add composer format/lint scripts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replace the hand-rolled Capsule bootstrap with Orchestra Testbench so the package is exercised through a real Laravel application: service provider registration, the database layer, and connection config. - Namespace tests and models under Lunar\Nestedset\Tests (PSR-4 autoload-dev) - Add a base TestCase that configures sqlite/mysql/pgsql connections from env vars, ready for a multi-engine CI matrix - Reset Postgres sequences after seeding rows with explicit ids - Add ServiceProviderTest covering the nestedSet()/dropNestedSet() macros - Drop phpunit.php; bootstrap from the Composer autoloader Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run the suite across all three engines via service containers, and add a dedicated coverage job (pcov + clover). Validated locally against MySQL 8 and PostgreSQL 16. Fix a test that ordered categories by a non-existent `title` column: SQLite and MySQL tolerate it, but PostgreSQL (correctly) rejects it. Order by `name` instead — the assertions are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add parameter, return and property types throughout the library now that the package requires PHP 8.3+, and remove docblocks made redundant by them. No behavioural change. Framework-override signatures are kept compatible with the installed Laravel version (some params left untyped where the parent declares none). A few inaccurate upstream docblocks were corrected to match real return types (e.g. moveNode returns bool, columnPatch returns Expression, getMissingParentQuery returns a base query builder). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Analyse src/ at level 5 with a baseline capturing the pre-existing trait-analysis findings (PHPStan's known limitation when a trait is analysed outside its host model). New code is held to level 5. Add a composer `analyse` script and a CI static-analysis job that also enforces Pint formatting. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a Contributing section to the README covering the test/lint/analyse scripts and multi-engine testing, plus CONTRIBUTING.md and CHANGELOG.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintenance pass now that the package is Lunar-maintained. All commits are scoped one-per-change for review.
What's in here
NestedSet::isNode()(array)$node) instead of used traits, so it returnedfalsefor every genuine node. Now usesclass_uses_recursive(); regression test added.format/lintcomposer scripts.nestedSet()/dropNestedSet()) now covered.src/; ~430 lines of redundant docblocks removed. Corrected several inaccurate upstream docblocks (e.g.moveNodereturnsbool,columnPatchreturnsExpression). No behavioural change.analysescript + CI job.CONTRIBUTING.md,CHANGELOG.md.composer.lockNotably
Testing across all three engines wasn't just box-ticking — PostgreSQL surfaced a real latent test bug (a test ordered
categoriesby a non-existenttitlecolumn; SQLite/MySQL silently tolerate it, Postgres rejects it). Fixed in the CI commit. Validated locally against MySQL 8 and PostgreSQL 16 via Docker.Verification
Out of scope (deliberately)
bigIntegerfor_lft/_rgt— left as-is per discussion.🤖 Generated with Claude Code