Create new package for reusable utilities#35
Conversation
📝 WalkthroughWalkthroughThis PR introduces a new PHP utility package ( Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
34-38:⚠️ Potential issue | 🟠 MajorCache keys are too coarse for this matrix.
Line 37 and Line 76 omit
matrix.php-version, and the package cache never keys off the package’s own dependency definition. Since/packages/**/composer.lockis gitignored, apackages/<package>/composer.jsonchange will keep reusing the old vendor cache. Include the PHP version and a per-package dependency hash in these keys.Also applies to: 73-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 34 - 38, The cache key for the actions/cache step is too coarse: update the cache key expression used in the actions/cache run (the key currently like ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }}) to include the matrix.php-version variable and a per-package dependency fingerprint so package-level changes bust the cache (e.g. include matrix.php-version and hashFiles over package manifests such as packages/**/composer.lock and packages/**/composer.json); apply the same change to both occurrences of the actions/cache key in the workflow so the cache is keyed by PHP version and per-package dependency hashes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/utils/README.md`:
- Around line 7-8: Update the two markdown badges that currently reference
"LacusSolutions/utils-php" so they point to the correct repository
"LacusSolutions/br-utils-php"; specifically replace the URL substrings in the
badge image/link markup (the occurrences of
https://github.com/LacusSolutions/utils-php and the license link to
.../utils-php/blob/main/LICENSE) with the corresponding
https://github.com/LacusSolutions/br-utils-php and
.../br-utils-php/blob/main/LICENSE so the Last Update and Project License badges
resolve to the correct repo.
In `@packages/utils/src/SequenceGenerator.php`:
- Around line 31-45: In SequenceGenerator::generate validate the $size argument
at the start and throw an InvalidArgumentException for negative values instead
of letting the loop be skipped; perform the check before selecting $chars (i.e.,
at the top of generate) and include a clear message (e.g., "Size must be
non-negative") so callers get a fast fail; leave zero allowed (returns empty) if
that behavior is desired.
In `@packages/utils/src/TypeDescriber.php`:
- Around line 28-31: The public static method describe currently exposes the
internal recursion flag $inArray; refactor by making describe(mixed $value):
string the public API and moving the boolean parameter into a private helper
(e.g., private static function describeInternal(mixed $value, bool $inArray):
string). Implement describe to delegate to describeInternal($value, false), and
update the internal caller that previously used describe(..., true) to call
describeInternal(..., true) instead (search for calls to describe passing true).
Ensure all logic that depended on $inArray remains in describeInternal and
remove the boolean from the public signature.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 34-38: The cache key for the actions/cache step is too coarse:
update the cache key expression used in the actions/cache run (the key currently
like ${{ runner.os }}-php-${{ hashFiles('**/composer.lock') }}) to include the
matrix.php-version variable and a per-package dependency fingerprint so
package-level changes bust the cache (e.g. include matrix.php-version and
hashFiles over package manifests such as packages/**/composer.lock and
packages/**/composer.json); apply the same change to both occurrences of the
actions/cache key in the workflow so the cache is keyed by PHP version and
per-package dependency hashes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71bdaf98-528c-4273-8ec4-b731a4bf5e69
📒 Files selected for processing (17)
.github/workflows/ci.yml.github/workflows/license-update.yml.github/workflows/subtrees-sync.yml.github/workflows/subtrees-tags-sync.ymlpackages/utils/LICENSEpackages/utils/README.mdpackages/utils/composer.jsonpackages/utils/phpunit.xmlpackages/utils/src/HtmlUtils.phppackages/utils/src/SequenceGenerator.phppackages/utils/src/SequenceType.phppackages/utils/src/TypeDescriber.phppackages/utils/src/UrlUtils.phppackages/utils/tests/HtmlUtilsTest.phppackages/utils/tests/SequenceGeneratorTest.phppackages/utils/tests/TypeDescriberTest.phppackages/utils/tests/UrlUtilsTest.php
Summary by CodeRabbit
Release Notes
New Features
Chores