8000 [Validator] Migrate File and Image constraints to attributes by derrabus · Pull Request #38410 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Validator] Migrate File and Image constraints to attributes #38410

New issue 8000

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

Merged
merged 1 commit into from
Oct 6, 2020

Conversation

derrabus
Copy link
Member
@derrabus derrabus commented Oct 4, 2020
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? no
Tickets #38096
License MIT
Doc PR TODO with symfony/symfony-docs#14305

I have migrated a lot of the constraints already and am preparing a big PR with them at the moment. I decided to pull this part out because it might raise some discussion.

This PR enables the File and Image constraints to be used as attributes. Especially the constructor signature of the Image constraint has grown pretty large this way. This by itself should be a big problem, if we don't expect the constructor to be called with ordered parameters by userland code. But it shows that the constraints have grown a bit too large. We might want to consider to split it.

@derrabus derrabus force-pushed the improvement/file-image-constraints branch from 9ad81f9 to d8c1869 Compare October 4, 2020 19:22
@nicolas-grekas nicolas-grekas added this to the next milestone Oct 5, 2020
@fabpot
Copy link
Member
fabpot commented Oct 6, 2020

Thank you @derrabus.

@fabpot fabpot merged commit 2f925df into symfony:master Oct 6, 2020
@derrabus derrabus deleted the improvement/file-image-constraints branch October 6, 2020 07:51
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.2 Oct 7, 2020
@fabpot fabpot mentioned this pull request Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0