enh: Add phpmd, phpstan and phpcs - #11
kofimokome wants to merge 2 commits into
Conversation
Code Review SummaryThis PR introduces a robust CI/CD pipeline for linting, static analysis (PHPStan), mess detection (PHPMD), and code style (PHPCS/Pint). It also updates various dependencies in 🚀 Key Improvements
💡 Minor Suggestions
🚨 Critical Issues
|
| echo round(\$coverage, 2); | ||
| ") | ||
| echo "Code coverage: $COVERAGE%" | ||
| MIN_COVERAGE=59.1 |
There was a problem hiding this comment.
The minimum coverage threshold is set to a very specific value (59.1). It is recommended to round this to an integer or use a more standard threshold (e.g., 60 or 80) to avoid failures on minor, non-logical line shifts.
| MIN_COVERAGE=59.1 | |
| MIN_COVERAGE=60 |
| use Illuminate\Database\Eloquent\Relations\MorphTo; | ||
| use Illuminate\Database\Eloquent\SoftDeletes; | ||
|
|
||
| use Workbench\App\Models\User; |
There was a problem hiding this comment.
A model within the src/ directory (package core) should not depend on Workbench\App\Models\User. This creates a circular dependency or assumes the consumer has the same namespace. Use the config or a contract for the User model.
| use Workbench\App\Models\User; | |
| use Illuminate\Foundation\Auth\User; |
| php-lint-sniffs: | ||
| name: PHP (Sniffs) | ||
| runs-on: ubuntu-latest | ||
| # timeout-minutes: 20 |
There was a problem hiding this comment.
Uncomment the timeout or remove it if not needed. It's better to have a reasonable timeout (e.g., 10 minutes) to prevent hung actions from consuming billing minutes.
| # timeout-minutes: 20 | |
| timeout-minutes: 10 |
| "vendor/bin/phpmd src text phpmd.xml" | ||
| ], | ||
| "phpstan": "phpstan analyse src --memory-limit=2048M --level=5", | ||
| "phpcs": "phpcbf src", |
There was a problem hiding this comment.
The phpcs script is configured to use phpcbf, which is a fixer, not a checker. This is confusing as the script name implies checking. Usually, phpcs should be for checking and phpcbf for fixing.
| "phpcs": "phpcbf src", | |
| "phpcs:fix": "phpcbf src", |
| 'invited_by' => $inv->invitedBy?->name, | ||
| 'expires_at' => $inv->expires_at, | ||
| 'created_at' => $inv->created_at, | ||
| /** |
There was a problem hiding this comment.
Instead of using @phpstan-ignore-next-line, you can improve the type safety of the collection. Using map(fn (WorkspaceInvitation $inv) => ...) inside a Laravel collection after a query should work correctly if PHPDocs are defined on the model or via generics in the get() method.
| /** | |
| ->map(function (WorkspaceInvitation $inv) { |
No description provided.