-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Add rector config #48223
base: master
Are you sure you want to change the base?
Add rector config #48223
Conversation
Only applying on apps for now, and only minimal type coverage level. Signed-off-by: Côme Chilliet <[email protected]>
Signed-off-by: Côme Chilliet <[email protected]>
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.
Can you add a script like https://github.com/nextcloud/groupfolders/blob/532f734b9d729a3c29a7371faf0bb85a24e0b311/composer.json#L26? This makes it a lot easier to use rector.
->withSkip([ | ||
__DIR__ . '/apps/*/3rdparty/*', | ||
__DIR__ . '/apps/*/build/stubs/*', | ||
__DIR__ . '/apps/*/composer/*', | ||
__DIR__ . '/apps/*/config/*', | ||
]) |
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.
Please put this into the first commit
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.
Psalm found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
|
||
return RectorConfig::configure() | ||
->withPaths([ | ||
__DIR__ . '/apps', |
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.
Make sure to exclude any git ignored paths like cs-fixer
Summary
And so it begins…
Long term goal is to apply rectors from https://github.com/nextcloud-libraries/rector to core code. We’ll need to be extra careful with folders like lib/public because they use some deprecated stuff on purpose I think.
So not sure if the best way forward is to add more rules or more folders.
Checklist