Skip to content

[5.0] Make use of new resolvingType method #6788

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.

Sign up for GitHub

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

Jump to bottom
Closed
hannesvdvreken wants to merge 1 commit into from
Closed

[5.0] Make use of new resolvingType method #6788

hannesvdvreken wants to merge 1 commit into from

Conversation

@hannesvdvreken
Copy link
Contributor

@hannesvdvreken hannesvdvreken commented Dec 24, 2014

Follow-up PR of #6724

Includes

  • More method documentation for new methods
  • Methods now also added to the Container contract
  • Made use of the new $container->resolvingType method to clean up ValidationServiceProvider and FormRequestServiceProvider.

@hannesvdvreken hannesvdvreken changed the title Make use of new resolvingType method [5.0] Make use of new resolvingType method Dec 24, 2014
GrahamCampbell
GrahamCampbell reviewed Dec 24, 2014
View reviewed changes
src/Illuminate/Validation/ValidationServiceProvider.php Outdated
Copy link
Collaborator

@GrahamCampbell GrahamCampbell Dec 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please do not use a leading slash. Please remove it.

Copy link
Contributor Author

@hannesvdvreken hannesvdvreken Dec 24, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done. ;-)

@hannesvdvreken
Copy link
Contributor Author

hannesvdvreken commented Dec 25, 2014

I'm thinking about rewriting the resolving and afterResolving methods to accept just a callback. If that callback's first argument is type hinted, then use that to only execute that callback when a resolved object is of that type. It removes the need for resolvingType, resolvingAny and it's after-variants.

GrahamCampbell
GrahamCampbell reviewed Dec 25, 2014
View reviewed changes
src/Illuminate/Validation/ValidationServiceProvider.php Outdated
Copy link
Collaborator

@GrahamCampbell GrahamCampbell Dec 25, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imports need to be sorted by length

Copy link
Contributor Author

@hannesvdvreken hannesvdvreken Dec 26, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're unused imports even. Forgot to remove them again.

@GrahamCampbell
Copy link
Collaborator

GrahamCampbell commented Dec 26, 2014

Please send your short array syntax changes in a different pull.

@hannesvdvreken
Copy link
Contributor Author

hannesvdvreken commented Dec 26, 2014

@GrahamCampbell does this "require changes"?

@hannesvdvreken hannesvdvreken mentioned this pull request Dec 26, 2014
Merged
@taylorotwell
Copy link
Member

taylorotwell commented Jan 2, 2015

Rebase?

@hannesvdvreken
Copy link
Contributor Author

hannesvdvreken commented Jan 4, 2015

@taylorotwell rebased

@GrahamCampbell
Copy link
Collaborator

GrahamCampbell commented Jan 4, 2015

👍

@hannesvdvreken
Copy link
Contributor Author

hannesvdvreken commented Jan 6, 2015

Ping

@taylorotwell
Copy link
Member

taylorotwell commented Jan 8, 2015

Is this needed after merging that other PR?

@hannesvdvreken
Copy link
Contributor Author

hannesvdvreken commented Jan 8, 2015

Nope

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@hannesvdvreken @GrahamCampbell @taylorotwell