-
-
Notifications
You must be signed in to change notification settings - Fork 0
feat(dev): Add php-forge/coding-standard to development dependencies for code quality checks.
#22
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.
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
Conversation
…s for code quality checks.
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughUpdates to coding‑standard tooling and documentation: StyleCI rule Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📓 Common learnings⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✏️ Tip: You can disable this entire section by setting 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
composer.json (1)
49-52: Theecsandrectorscripts will fail—php-forge/coding-standardis not a valid Packagist package.The composer.json requires
php-forge/coding-standard(^0.1) as a replacement, but this package does not exist on Packagist. As a result:
- The package cannot be resolved in composer.lock
- The transitive dependencies (if any) are not installed
- The binaries
./vendor/bin/ecsand./vendor/bin/rectorare missing- Running the
ecsandrectorscripts (lines 49 and 52) will failEither restore the original
symplify/easy-coding-standardandrector/rectorpackages, or replace with a valid package that provides these tools.
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Line 5: Update the CHANGELOG.md entry that currently reads "Bug `#17`: Add
`php-forge/coding-standard` to development dependencies for code quality checks
(`@terabytesoftw`)" to use the correct type/prefix for a new feature/enhancement
(e.g., change "Bug `#17`" to "Enhancement `#17`" or "Feature `#17`") so the changelog
matches the PR metadata; locate the exact string in CHANGELOG.md and replace the
prefix only, keeping the rest of the entry intact.
In `@rector.php`:
- Around line 5-8: Confirm the correct package name/path for the external config
(the referenced php-forge/coding-standard package seems incorrect) and update
the import string in the returned static function accordingly; if the external
config is optional, guard the call to $rectorConfig->import by checking that the
target file exists (e.g., is_file on the resolved path) and skip or log a clear
message when missing so Rector won't fail at runtime when the
vendor/…/rector.php file is absent.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (8)
.styleci.ymlCHANGELOG.mdREADME.mdcomposer.jsondocs/development.mddocs/testing.mdecs.phprector.php
🧰 Additional context used
🧠 Learnings (6)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: tests/Fallback/AssetFallbackTest.php:158-189
Timestamp: 2026-01-23T11:22:07.261Z
Learning: User terabytesoftw prefers not to use aliases when importing classes in PHP. They are comfortable with either fully-qualified class names or direct imports without aliases.
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
📚 Learning: 2026-01-23T14:44:05.604Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: ecs.php:81-95
Timestamp: 2026-01-23T14:44:05.604Z
Learning: In symplify/easy-coding-standard (ECS), the `withPhpCsFixerSets()` method converts PHP-CS-Fixer rule set names to camelCase parameter names by removing special characters. For example, `PER-CS3x0` becomes `perCS30` (not `perCS3x0`), and `PER-CS2x0` becomes `perCS20`.
Applied to files:
ecs.php
📚 Learning: 2025-09-29T14:58:04.095Z
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 67
File: .github/workflows/phpstan.yml:13-16
Timestamp: 2025-09-29T14:58:04.095Z
Learning: In the php-forge/actions repository, the maintainer prefers using "composer update" as the default command in CI workflows to ensure the latest dependencies are always installed, rather than using "composer install" for reproducibility.
Applied to files:
docs/testing.mdcomposer.json
📚 Learning: 2026-01-23T11:09:15.771Z
Learnt from: terabytesoftw
Repo: php-forge/foxy PR: 115
File: composer.json:68-77
Timestamp: 2026-01-23T11:09:15.771Z
Learning: In the php-forge organization repositories, terabytesoftw prefers using the main branch reference in composer.json sync-metadata scripts when downloading configuration files from external template repositories, rather than pinning to specific commit SHAs or tags, even though pinning provides more stability and prevents unexpected changes.
Applied to files:
docs/development.mdcomposer.json
📚 Learning: 2025-08-18T15:43:30.996Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: composer.json:13-16
Timestamp: 2025-08-18T15:43:30.996Z
Learning: Testing helper libraries (packages with "testing" or "support" in keywords/description that provide test utilities) should include PHPUnit and other testing frameworks in the main "require" section rather than "require-dev", as consuming packages need these dependencies to use the testing utilities.
Applied to files:
composer.json
📚 Learning: 2025-08-18T16:13:31.606Z
Learnt from: terabytesoftw
Repo: php-forge/support PR: 11
File: phpstan.neon:1-3
Timestamp: 2025-08-18T16:13:31.606Z
Learning: In the php-forge/support project, PHPStan is intended to be used as a PHAR installation rather than as a Composer dependency, even though the composer.json was updated to include phpstan/phpstan as a dependency.
Applied to files:
composer.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: phpunit / PHP 8.5-ubuntu-latest
- GitHub Check: phpunit / PHP 8.2-windows-2022
- GitHub Check: phpunit / PHP 8.3-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-ubuntu-latest
- GitHub Check: phpunit / PHP 8.3-windows-2022
- GitHub Check: linter / Super Linter
- GitHub Check: mutation / PHP 8.5-ubuntu-latest
🔇 Additional comments (8)
docs/development.md (1)
11-11: LGTM!The documentation updates are consistent with the PR changes:
- The simplified
composer sync-metadatacommand syntax is valid (equivalent tocomposer run sync-metadata)- The Updated Files table correctly reflects the addition of
.styleci.ymland removal ofecs.php/rector.phpfrom the sync-metadata scriptAlso applies to: 23-23
.styleci.yml (1)
58-58: LGTM!The
phpdoc_param_orderrule is a sensible addition that enforces PHPDoc@paramtags to match the order of function parameters. The placement maintains the alphabetical ordering convention used in this configuration file.README.md (1)
302-303: LGTM!The emoji prefixes add helpful visual context to distinguish the documentation links. The underlying links remain correct.
composer.json (2)
19-19: LGTM - Development dependency added correctly.The
php-forge/coding-standardpackage is properly added torequire-devwith a caret constraint allowing minor version updates.
54-61: LGTM - Sync-metadata scripts updated consistently.The script changes align with the PR's shift to using
.styleci.ymlfor style configuration, and the removal ofecs.phpandrector.phpfetches is consistent with the new approach of delegating to thephp-forge/coding-standardpackage.Based on learnings, the use of main branch references in sync-metadata scripts aligns with the maintainer's preference for this organization.
ecs.php (1)
5-13: LGTM - Clean delegation to centralized coding standards.The configuration cleanly delegates to the
php-forge/coding-standardpackage, which centralizes rule management across projects. The builder pattern allows project-specific path configuration while inheriting standardized rules.Verify that the
php-forge/coding-standardpackage exports the expectedecs.phpconfiguration file:#!/bin/bash # Check if the coding-standard package contains the expected ECS config # First, check if the package is available and examine its structure # Search for any documentation about the config structure fd -t f -e md . | xargs grep -l "ecs.php" 2>/dev/null || echo "No documentation found referencing ecs.php" # Check if composer.lock exists and contains the package if [ -f composer.lock ]; then jq '.packages[] | select(.name == "php-forge/coding-standard") | {name, version}' composer.lock 2>/dev/null || \ jq '.packages-dev[] | select(.name == "php-forge/coding-standard") | {name, version}' composer.lock 2>/dev/null fidocs/testing.md (2)
27-81: Docs update to Composer shorthand and extra-args examples looks good.The switch to
composer <script>and the updated forwarding examples are clear and consistent.
12-20: Thecomposer rectorscript is properly defined incomposer.json, so the documentation is accurate and will not mislead users.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Pull Request