Skip to content
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

Fix Phpdoc #7598

Merged
merged 1 commit into from
Nov 7, 2021
Merged

Fix Phpdoc #7598

merged 1 commit into from
Nov 7, 2021

Conversation

VincentLanglet
Copy link
Member

Subject

BC

Should solve the psalm issue in https://github.com/sonata-project/SonataDoctrineORMAdminBundle/pull/1573/files

Changelog

### Fixed
- Filter interface phpdoc

@@ -90,14 +90,14 @@ public function getParentAssociationMappings(): array;
/**
* @throws \RuntimeException if the `field_mapping` option is not set
*
* @return array<string, string> field mapping
* @return array<string, mixed> field mapping
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add an array shape signature here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say no because it's persistence bundle specific.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even if it is persistence bundle specific, this interface defines the contract. What options are normally returned here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would recommend to discuss this in an issue like #6274 and not block this PR which is just a phpdoc fix.

The option returns can be anything, based on what the persistence bundle use. DoctrineORM and DoctrineMongo doesn't use the same key or values. For instance targetDocument is used in mongo but not in orm. And I could write my own persistence bundle with some others keys.

So currently any keys or values are allowed and IMHO should be allowed for now until something better is proposed like an Object

@VincentLanglet VincentLanglet requested a review from a team November 7, 2021 18:06
@VincentLanglet VincentLanglet merged commit 5a7d134 into sonata-project:4.x Nov 7, 2021
@VincentLanglet VincentLanglet deleted the fixPh branch November 7, 2021 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants