Supported "Marking" enum php #50311
Supported "Marking" enum php #50311
Conversation
|
Hey! I see that this is your first PR. That is great! Welcome! Symfony has a contribution guide which I suggest you to read. In short:
Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change. When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor! I am going to sit back now and wait for the reviews. Cheers! Carsonbot |
|
Places are strings in the workflow component. See #44211 for the discussion about the support for enums in that component. |
There was a problem hiding this comment.
Thank you for your attempt to improve Symfony. Please implement all changes in a backwards-compatible manner. Here's an overview of what you can and cannot change: https://symfony.com/doc/current/contributing/code/bc.html#working-on-symfony-code
| * @return void | ||
| */ | ||
| public function mark(string $place) | ||
| public function mark(string|\UnitEnum $place): void |
There was a problem hiding this comment.
Note that this method is not final and could be overridden downstream. Widening the parameter type and narrowing the native return type are both breaking changes.
There was a problem hiding this comment.
I propose to make Marking.php the final class
There was a problem hiding this comment.
We cannot do that now, as making a class final is also a breaking change. So we can at most flag it as final using PHPdoc and make it final in 7.0. But that would also mean that we probably need to introduce a MarkingInterface and update the rest of the component to use that typehint, as otherwise this is fully closed for extension.
There was a problem hiding this comment.
Yes, you're right, it's better to leave it like that than to make many changes in favor of MarkingInterface
Thank you, I have read. Nevertheless, the desire to use enum is still present, and this decision turned out to be not so cumbersome and critical. |
|
"Desire" to use a PHP feature in your own code does not exempt your contribution from respecting required rules of all contributions submitted to Symfony (as we cannot accept it if it breaks BC) |
I understand you and completely agree. Nevertheless, this feature will be useful to many users. |
|
Closing as we don't merge PRs with BC breaks. |
I would like to be able to use the getters and setters of my entity with the enum type without custom implements
MarkingStoreInterface.Because php does not support enum in associative arrays, and
\WeakMapis not suitable for this case, it was decided to make a methodgetEnumPlaces