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

symfony uuid type descriptor #639

Draft
wants to merge 1 commit into
base: 2.0.x
Choose a base branch
from
Draft

symfony uuid type descriptor #639

wants to merge 1 commit into from

Conversation

alcohol
Copy link

@alcohol alcohol commented Jan 21, 2025

References #637

@alcohol alcohol force-pushed the 2.0.x branch 4 times, most recently from ea7e7fb to 9540f53 Compare January 21, 2025 12:37
@ondrejmirtes
Copy link
Member

No idea why it's failing. Try removing the dependencies from require-dev you just added.

@alcohol
Copy link
Author

alcohol commented Jan 21, 2025

I think it's failing cause a test uses a class from symfony/validator but that component is not in the require-dev list.

@ondrejmirtes
Copy link
Member

Also - the descriptor has to be registered, and also - add a test that shows what problem it's solving. Descriptors are already being tested in EntityColumnRuleTest.

@alcohol
Copy link
Author

alcohol commented Jan 21, 2025

That fixed most existing failures; except for deprecations on the very latest version.

@alcohol
Copy link
Author

alcohol commented Jan 21, 2025

@ondrejmirtes can I add the test inside the same class? Am wondering since both Ramsey and Symfony register a 'uuid' type and the one for Ramsey is already tested/used in that class.

@ondrejmirtes
Copy link
Member

Yeah, sure.

@alcohol
Copy link
Author

alcohol commented Jan 21, 2025

Yeah that didn't work; the error is for the Ramsey uuid type then, not Symfony.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants