- Issue created by @DieterHolvoet
- Status changed to Needs review
about 2 months ago 9:29am 16 July 2024 - Status changed to Needs work
about 2 months ago 10:21am 16 July 2024 - 🇦🇺Australia mstrelan
Why remove the return type instead of just making it nullable?
- First commit to issue fork.
- Status changed to Needs review
about 2 months ago 10:47am 16 July 2024 - 🇮🇳India manish-31
Yes @mstrelan, IMO it would be better.
Added nullable return type. Needs review. Thanks! - 🇧🇪Belgium DieterHolvoet Brussels
None of the parent classes/interfaces have a return type yet, so leaving it out until they do seems safer to me.
- Status changed to Needs work
about 2 months ago 1:50pm 16 July 2024 - 🇺🇸United States smustgrave
The inherited doc says this only returns an int so if that's incorrect that should be fixed as well.
- Status changed to Needs review
about 2 months ago 2:08pm 16 July 2024 - 🇧🇪Belgium DieterHolvoet Brussels
There are existing issues dealing with that, I referenced them in the related issue list.
- 🇺🇸United States smustgrave
Not sure we should compound on it. Definitely -1 for changing return type but leaving docs wrong.
- 🇧🇪Belgium DieterHolvoet Brussels
I feel like it would take a long time to change EntityInterface::save() to either return nothing or always return an int, because there are a lot of implementations of that method. In order to fix this bug as fast as possible, and to not confuse people or tools like PHPStan looking at the method docs any longer, I propose we just make the return type nullable in the docs. I'll add that to the MR.
- Status changed to Needs work
about 1 month ago 3:41pm 1 August 2024 - 🇧🇪Belgium DieterHolvoet Brussels
After further investigation I found that this specific issue comes from the no_entity_view_display module not returning anything in the
save()
method. I'll transfer the issue to that project. - Merge request !5Fix EntityViewDisplayStorage violating method return types → (Open) created by DieterHolvoet
- Status changed to Needs review
about 1 month ago 3:46pm 1 August 2024