Skip to content

[Validator] Add Validation::createCallable() #15600

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

Merged
merged 1 commit into from
Aug 9, 2021

Conversation

W0rma
Copy link
Contributor

@W0rma W0rma commented Aug 7, 2021

@carsonbot
Copy link
Collaborator

Hey!

Oh no, it looks like you have made this PR towards a branch that is not maintained anymore. :/
Could you update the PR base branch to target one of these branches instead? 4.4, 5.3, 5.4, 6.0.

Cheers!

Carsonbot

@carsonbot carsonbot added this to the 5.1 milestone Aug 7, 2021
@W0rma W0rma force-pushed the validation-callables branch from d65b84e to 1539bca Compare August 7, 2021 19:53
@W0rma W0rma requested a review from OskarStark as a code owner August 7, 2021 19:53
@W0rma W0rma changed the base branch from 5.1 to 5.3 August 7, 2021 19:54
@W0rma
Copy link
Contributor Author

W0rma commented Aug 7, 2021

@carsonbot Thank you for the hint. I rebased against 5.3

@wouterj
Copy link
Member

wouterj commented Aug 8, 2021

Hi @W0rma! This is a great contribution, this feature was missing docs for quite some time.

I have some suggestions on the location/content of this article. Please let me know if you don't agree or if you don't have the time to make the changes, I'm sure we can get this merged in any case :)

  • We're trying to reduce the number of small "sub articles" and move things into the main article. What do you think about moving this contribution to validation.rst, as a new section under "The Basics of Validation"?
  • I think we can be a bit more to-the-point in the main validation article. E.g. something like:
    The ``Validation`` also allows you to create a closure to validate values
    against a set of constraints (e.g. in ... or ...):
    
    ``createCallable()``
       This returns a closure that throws ... when the constraints aren't matched.
    ``createValidCallable()``
       ...
  • Then, we can advocate how nicely this plays with other components in their docs. E.g. add a little tip with your examples to the options resolver and console question helper docs.

@W0rma W0rma force-pushed the validation-callables branch from 1539bca to b89e08c Compare August 9, 2021 09:14
@W0rma W0rma force-pushed the validation-callables branch from b89e08c to 6cc9f38 Compare August 9, 2021 09:21
@W0rma
Copy link
Contributor Author

W0rma commented Aug 9, 2021

@wouterj Thank you for the suggestions. I think they are all very good ideas and therefore I applied them to this PR.

@javiereguiluz javiereguiluz modified the milestones: 5.1, 5.3 Aug 9, 2021
@javiereguiluz javiereguiluz merged commit 2754496 into symfony:5.3 Aug 9, 2021
@javiereguiluz
Copy link
Member

@W0rma great contribution! Thanks a lot!

While merging we added some references to better link sections between them. See 4405199.

@ngrie
Copy link

ngrie commented Aug 13, 2021

@W0rma @javiereguiluz I just stumbled over this in the docs and noticed that the mentioned method createValidCallable() does not exist, probably the createIsValidCallable() method should be referenced instead. Or am I missing something?

@javiereguiluz
Copy link
Member

@ngrie good catch! We've just fixed that in 408fd80 and 4b01042

@W0rma W0rma deleted the validation-callables branch October 1, 2024 05:38
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.

5 participants