Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[FWB][Serializer][Form][Validator] Uid integration#36317
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
maxhelias left a comment
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 love the idea very much but I don't agree with the form integration. Normally, you should never have uid fields in a form. Otherwise a new doctrine type in the bridge would be really nice.
| $io =newSymfonyStyle($input,$output); | ||
| if (!class_exists(AbstractUid::class)) { | ||
| thrownew \RuntimeException('Unable to execute this command as the Symfony Uid Component is not installed.'); |
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.
You should remove the service in the FrameworkExtension file when theAbstractUid doesn't exist instead of throwing an exception
| </service> | ||
| <serviceid="serializer.normalizer.uid"class="Symfony\Component\Serializer\Normalizer\UidNormalizer"> | ||
| <argument>%kernel.debug%</argument> |
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.
You should remove this argument
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.
Done.
maxhelias commentedApr 4, 2020
BTW, let me know when you have finished, i could test it |
ro0NL commentedApr 4, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
i tend to prefer concrete UUID, ULID, ... validators. It allows for more fine grained control per type. E.g. ensure a specific UUID version maybe. I'd also think we should either support sf/uid as well as ramsey/uuid, but not an issue for me. Im not sure about the form type either, nor the console command. |
dunglas commentedApr 5, 2020
👍 for the serializer part I've not reviewed other parts. |
guillbdx commentedApr 6, 2020 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Hi@ro0NL@dunglas , that is the case, the Another possibility would be to have 2 options :
Or another possibility: 2 different constraints, 1 ULID constraint and 1 UUID constraint, with a What do you think is better? |
xabbuh commentedApr 7, 2020
Maybe you could split the PR into smaller ones so that people can focus on the components they know best? |
ro0NL commentedApr 7, 2020
i strongly prefer constraints per case. |
guillbdx commentedApr 9, 2020
I've split this PR in 3 smaller PR, one per component:
I'm closing this PR. |
Uh oh!
There was an error while loading.Please reload this page.
Integration of the Uid Component to Symfony stack: