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

[5.5] Re-use implicit binding resolving logic that is already present on model #20618

Merged
merged 2 commits into from
Aug 18, 2017
Merged

Conversation

balping
Copy link
Contributor

@balping balping commented Aug 17, 2017

This was my original intention in #20521

However I was inattentive there: when I moved the resolving logic to eloquent model, I made only one replacement, while 3 could have been done. (+ I was wrong on calling that implicit binding while that was about explicit binding without callback function, ie. Route::model)

This PR fixes that by reusing the resolving logic already being present in eloquent's resolveRouteBinding

Note that all three occurrences used to implement exactly the same logic, so this PR, together with the merged original one, removes code repetition too.

There was another related PR where Taylor Otwell said

We're not changing anything else about this

But I hope he meant the controversy around UrlRoutable. This PR has nothing to do with that, it simply removes duplicated code and completes my original intention of making it possible to alter implicit route model binding resolution by overriding a method on a model.

@balping balping changed the title [5.5] Re-use implicit binding resolving logic that is already on model [5.5] Re-use implicit binding resolving logic that is already present on model Aug 17, 2017
@taylorotwell taylorotwell merged commit 2626de6 into laravel:master Aug 18, 2017
balping added a commit to balping/laravel-hashslug that referenced this pull request Aug 18, 2017
Because it has been possible to do this in a clear way, since this PR was merged: laravel/framework#20618
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.

None yet

2 participants