-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
PHPDoc generation for models violates laravel_phpdoc_separation
StyleCI rule
#1288
PHPDoc generation for models violates laravel_phpdoc_separation
StyleCI rule
#1288
Comments
Personally I don't consider anything of this "official", there is no "official laravel code style" AFAICS, the best we've is https://github.com/matt-allan/laravel-code-style but that's not official either. Also: just because someone is using Laravel does not mean they're bound to using that code style. Same would be true if someone was Symfony but not their CS etc. (fun fact: at least when it comes to php-cs-fixer, there's an official Symfony code style rule set). A good project has fixing CI as part of their pipeline somewhere anyway, therefore I'd say: if you care about code style, make sure you have it in your pipeline and then it doesn't matter what ide-helper generated. If we change it, others might complain that we changed it. To me it's a non-issue, but you can of course try a PR if you want. |
This also conflicts with Laravel Pint now, maybe that is enough reason to fix it now? |
Personally (<- achtung: personal opinion only!): I don't see the point investing time here. Even though many use Laravel, does not mean everyone uses the same code style (be it laravel/pint , To me, the only true solution is: run an automatic code fixer as part of your CI/CD pipeline, then it's a non-issue. |
This package is specifically designed for Laravel, and Laravel does now have official style guidelines (since Pint), so I'd argue that the produced PHPDoc should adhere to the official style guidelines. Of course, not everyone will stay with the official default. But the majority will. And yes, a CI/CD pipeline also solves this issue, but I think it's a bad solution to require users to use a CI pipeline if they want to use this package without running into conflicts with the default Laravel installation. |
Any update on this? I see that the missing PR #1377 is not merged. But from the comments it seems to be rejected by the reviewers. Not been any update on that since feb 2023. Since then laravel 11 has shipped with laravel pint installed by default. That makes it three major versions (9, 10, 11) where it is included by default. I will +1 the arguments made in favor of adding this. If the package has an opt-out in the config it will also be compatible with people not using laravel pint preset or any other formatter. |
The PHPDoc generation for models (
php artisan ide-helper:models
) generates the following:However, this violates the laravel_phpdoc_separation StyleCI rule, which is a default rule for the StyleCI Laravel preset (which is used as a default for Laravel itself). There should be a linebreak between the
@property
declarations and@method
declarations, like this:The text was updated successfully, but these errors were encountered: