feat: convert into yii2-extensions/scaffold provider; centralize ECS, Rector, metadata and super-linter standards under src/.#4
Conversation
…S, Rector, metadata and super-linter standards under `src/`.
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis PR transforms the project into a Composer-distributed coding standard package with version-specific ECS and Rector configuration wrappers. A shared base configuration provides common rules, while wrapper files ( ChangesCoding Standard Distribution & Configuration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/linters/.gitleaks.toml:
- Around line 3-7: Replace the deprecated single-table header [allowlist] with
the array-of-tables header [[allowlists]] and move the existing keys
(description and paths) under that new table so the file uses the modern
gitleaks syntax; specifically, change the block beginning with [allowlist] to
start with [[allowlists]] and keep the description and paths array (including
the '''tests/support/data/.*\.php''' entry) unchanged so semantics are
preserved.
In @.prettierrc.json:
- Around line 9-20: The patterns in the "files" array currently use root-level
globs (e.g., "*.js", "*.ts", "*.vue") which won't match nested files; update
each pattern in that array to a recursive glob (replace "*.ext" with "**/*.ext")
so Prettier overrides apply to files in subdirectories (e.g., change "*.js" →
"**/*.js", "*.ts" → "**/*.ts", etc.).
In `@CHANGELOG.md`:
- Around line 8-12: Update the 0.2.0 changelog entry to explicitly mark the
change as breaking: modify the "## 0.2.0 Under development" section (the 0.2.0
entry) to replace or augment the current "- feat: convert into
`yii2-extensions/scaffold` provider..." line with a "- BREAKING CHANGE: ..."
prefix and add a short migration note explaining what consumers must change
(e.g., provider name change, config/key paths, or any renamed classes/APIs) so
users can follow upgrade steps; ensure the new text references the same 0.2.0
heading and mentions the concrete migration action(s) required.
In `@docs/installation.md`:
- Around line 24-26: The manual composer example places
"php-forge/coding-standard" under "require" but the earlier instruction uses dev
install; change the JSON key in the example from "require" to "require-dev" so
the package appears as a dev dependency (i.e., move "php-forge/coding-standard":
"^0.2" under "require-dev") to keep the docs consistent and prevent production
installs.
In `@rector.php`:
- Line 8: Add a clear comment in rector.php next to the import call
($rectorConfig->import(... 'rector-83.php')) instructing consumers to replace
that import with the version-specific file matching their project's PHP
constraint in composer.json; then update docblock examples in src/rector-81.php,
src/rector-82.php, src/rector-83.php and src/rector-84.php so they reference the
correct src/ paths (not config/) and ensure src/rector-82.php fixes the
erroneous import of rector-81.php to import the correct rector-82.php file and
any mismatched Set/Level references.
In `@src/ecs-81.php`:
- Around line 15-18: The example require path in src/ecs-81.php is pointing to
vendor/php-forge/coding-standard/config/ecs-81.php which doesn't exist; update
the require to reference the installed location under src (e.g.
vendor/php-forge/coding-standard/src/ecs-81.php) so the $builder = require ...
line resolves correctly (same fix applied as in src/ecs-84.php).
In `@src/ecs-83.php`:
- Around line 15-18: Update the require path used to load the ECS config in
src/ecs-83.php so it points to the package's src location rather than config;
change the string passed to the require for the $builder assignment (the line
with $builder = require ...) to
'/vendor/php-forge/coding-standard/src/ecs-83.php' (matching how src/ecs-81.php
and src/ecs-84.php reference the package) so the docblock/example uses the
correct installed path.
In `@src/ecs-84.php`:
- Around line 15-18: The example require path in the docblock is incorrect and
points to config/ecs-84.php which doesn't exist; update the require invocation
that assigns $builder (the line using require __DIR__ .
'/vendor/php-forge/coding-standard/config/ecs-84.php') to reference the
installed location under src (e.g. require __DIR__ .
'/vendor/php-forge/coding-standard/src/ecs-84.php') so the $builder variable is
loaded correctly and the subsequent call to $builder->withPaths([...]) works.
In `@src/rector-81.php`:
- Around line 21-23: Update the example import path in the docblock: the example
using $rectorConfig->import(__DIR__ .
'/vendor/php-forge/coding-standard/config/rector-81.php') is outdated after
centralizing files under src/; change it to point to the new location (e.g.
$rectorConfig->import(__DIR__ .
'/vendor/php-forge/coding-standard/src/rector-81.php') or the exact new path
where rector-81.php now lives) so the example reflects the relocated file in
rector-81.php.
In `@src/rector-82.php`:
- Around line 21-23: The import in the anonymous config function calls the wrong
wrapper file: update the string passed to $rectorConfig->import (currently
referencing 'rector-81.php') to reference the 8.2 wrapper ('rector-82.php') so
the config uses the correct Rector 8.2 rules; ensure the $rectorConfig->import
call now points to 'vendor/php-forge/coding-standard/config/rector-82.php'.
In `@src/rector-83.php`:
- Around line 30-35: The sets call is redundant: remove the explicit
SetList::PHP_83 entry from the $rectorConfig->sets([...]) call so only
LevelSetList::UP_TO_PHP_83 remains; locate the $rectorConfig->sets invocation
(currently listing LevelSetList::UP_TO_PHP_83 and SetList::PHP_83) and delete
the SetList::PHP_83 element, leaving a single LevelSetList::UP_TO_PHP_83 entry.
- Around line 21-24: The docblock example uses the wrong package path: update
the Rector config import in the anonymous return function so the call to
$rectorConfig->import(...) points to
'/vendor/php-forge/coding-standard/src/rector-83.php' (not
'/vendor/.../config/...'), i.e. edit the import argument inside the static
function where $rectorConfig->import is invoked to reference the src directory
so consumers won't get a fatal missing-file error.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 722dccd5-2aeb-4ba5-982a-be001f8c8cc6
⛔ Files ignored due to path filters (2)
docs/svgs/features-mobile.svgis excluded by!**/*.svgdocs/svgs/features.svgis excluded by!**/*.svg
📒 Files selected for processing (34)
.ecrc.gitattributes.github/linters/.codespellrc.github/linters/.gitleaks.toml.github/linters/.markdown-lint.yml.github/workflows/ecs.yml.gitignore.prettierignore.prettierrc.json.stylelintignoreCHANGELOG.mdREADME.mdcomposer-require-checker.jsoncomposer.jsondocs/configuration.mddocs/development.mddocs/examples.mddocs/installation.mddocs/testing.mdecs.phprector.phpsrc/Example.phpsrc/ecs-81.phpsrc/ecs-82.phpsrc/ecs-83.phpsrc/ecs-84.phpsrc/ecs.phpsrc/rector-81.phpsrc/rector-82.phpsrc/rector-83.phpsrc/rector-84.phpsrc/rector.phptests/ExampleTest.phptests/bootstrap.php
💤 Files with no reviewable changes (6)
- docs/examples.md
- docs/development.md
- docs/configuration.md
- tests/ExampleTest.php
- docs/testing.md
- src/Example.php
📜 Review details
⏰ 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). (1)
- GitHub Check: linter / Super Linter
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~174-~174: The official name of this software platform is spelled with a capital “H”.
Context: ...ker whitelist (project-specific) | | .github/linters/actionlint.yml | replace...
(GITHUB)
[uncategorized] ~175-~175: The official name of this software platform is spelled with a capital “H”.
Context: ... Super-Linter | | .github/linters/.codespellrc | replace...
(GITHUB)
[uncategorized] ~176-~176: The official name of this software platform is spelled with a capital “H”.
Context: ... | | .github/linters/.gitleaks.toml | replace...
(GITHUB)
[uncategorized] ~177-~177: The official name of this software platform is spelled with a capital “H”.
Context: ... | | .github/linters/.markdown-lint.yml | replace...
(GITHUB)
🪛 OpenGrep (1.20.0)
src/ecs-84.php
[ERROR] 23-23: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/ecs-81.php
[ERROR] 23-23: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/ecs-82.php
[ERROR] 23-23: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/rector-82.php
[ERROR] 28-28: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/rector-84.php
[ERROR] 28-28: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/rector-83.php
[ERROR] 28-28: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
ecs.php
[ERROR] 6-6: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/rector-81.php
[ERROR] 27-27: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
src/ecs-83.php
[ERROR] 23-23: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.
(coderabbit.file-inclusion.php-dynamic-include)
🔇 Additional comments (16)
.stylelintignore (1)
1-1: LGTM..gitignore (1)
31-32: LGTM.composer-require-checker.json (1)
1-3: LGTM..github/linters/.codespellrc (1)
1-2: LGTM..github/linters/.markdown-lint.yml (1)
1-5: LGTM..prettierignore (1)
1-18: LGTM..ecrc (1)
1-10: Looks good — exclude patterns are coherent and scoped.This exclusion list is consistent with the repo’s tooling/runtime boundaries.
.gitattributes (1)
25-37: Archive and merge attributes update looks consistent.Targeted
.githubexclusions plus retainedCHANGELOG.md merge=unionare sensible here.src/rector.php (1)
11-18: Base Rector config documentation is clear and matches implementation.The version-agnostic base plus wrapper layering guidance is well aligned with the config body.
src/rector-84.php (1)
27-35: Wrapper layering is implemented correctly.The base import + PHP 8.4 set layering matches the intended architecture.
src/ecs.php (1)
95-103: LGTM — explicit strict fixers correctly replace the deprecatedstrict: trueprepared set.The three extracted fixers (
DeclareStrictTypesFixer,StrictComparisonFixer,StrictParamFixer) match exactly what the deprecated set bundled, preserving behavior while giving consumers direct control over each rule.ecs.php (1)
5-13: LGTM — correct path and clean consumer pattern.The
src/path in Line 6 correctly matches where the versioned wrapper is installed, and the builder is properly extended with project-specific paths. This file also serves as the authoritative reference showingsrc/(notconfig/) as the correct consumer include path.composer.json (1)
28-51: LGTM — scaffold configuration is well-structured.Mode choices are appropriate:
"preserve"for consumer-owned configs (ecs.php,rector.php,composer-require-checker.json) avoids overwriting downstream customizations, and"append"for.gitignoreis the correct strategy for extending rather than replacing consumer ignore rules..github/workflows/ecs.yml (1)
22-23: LGTM — CI command is consistent with the updatedcomposer.jsonscripts.Using
check srcwithout--fixin CI is the correct pattern (report violations, don't silently mutate), and--ansiimproves log readability.src/ecs-82.php (1)
3-25: Looks good — clean versioned ECS wrapper.
strict_types, fixed local import, and PHP 8.2 migration-set layering are consistent and safe here.README.md (1)
29-199: Documentation restructure is clear and actionable.The config matrix + scaffold section reads consistently and aligns with the new provider model.
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Pull Request