-
Notifications
You must be signed in to change notification settings - Fork 43
TASK: Introduce PHP code linting / refactoring and fix affected code #608
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
base: main
Are you sure you want to change the base?
Conversation
After deployment this change first needs to be migrated with ``` ./flow nodemigration:execute 20250623134952 ``` Then remove the deprecated childnodes with ``` ./flow structureadjustments:fix —node-type Neos.MarketPlace:Version ```
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.
THanks for the effort!!! Cant say too much about the code
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.
Thanks a lot for taking care.
Looks great in general, I only found some smaller things that might be bugs.
Apart from that, this is more than just "Code linting" – which is great, but we should adjust the description slightly IMO
DistributionPackages/Neos.NeosConIo/Classes/Eel/FlowQueryOperations/SortMultipleOperation.php
Outdated
Show resolved
Hide resolved
DistributionPackages/Neos.NeosConIo/Classes/Eel/FlowQueryOperations/SortMultipleOperation.php
Show resolved
Hide resolved
DistributionPackages/Neos.NeosIo/Classes/Service/CrowdApiConnector.php
Outdated
Show resolved
Hide resolved
bootstrap-phpstan.php
Outdated
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.
I don't understand this. Isn't the root problem, that we are missing an explicit dependency on neos/media
(or rather neos/neos
?) even though we are using classes of those namespaces?
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.
I'm not quite sure.@mhsdesign fixed it like this in the core and I copied the file.
DistributionPackages/Neos.NeosIo/Classes/Fusion/FlowQueryOperations/CrowdGroupsOperation.php
Show resolved
Hide resolved
DistributionPackages/Neos.NeosIo/Classes/Fusion/FlowQueryOperations/CrowdGroupsOperation.php
Outdated
Show resolved
Hide resolved
DistributionPackages/Neos.NeosIo/Classes/Fusion/FlowQueryOperations/CrowdUserOperation.php
Outdated
Show resolved
Hide resolved
FEATURE: Simplify marketplace structure
dammit I accidentally already merged the marketplace refactoring in here |
Nevermind. Let's just fix composer.lock and get this merged! |
Linting of the migration will be separately solved with Sebobo/Shel.Neos.Schema#29 |
Yaml is now also linting fine |
NodeType and configuration schema linting
PHPStan level 8