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

[9.x] Add giveConfig to ContextualBindingBuilder #39819

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

simensen
Copy link
Contributor

This is a follow-up to #36241. I had intended to work on the contract eventually and thought maybe it would just get added eventually by someone else.

I believe this may require additional work on the contract (?) that I'll do if this gets approved. Would just need to know which branch to target that change on. I can also do the docs on this if it looks like this will be able to be merged. :)

This afternoon I decided to go ahead and add it. In the process, I discovered I'm not the first to send a PR for this addition! Unfortunately, both #36361 and #38925 were rejected.

I'm going to send my PR anyway to open the discussion again. In my initial PR, it was not stated that this was syntactical sugar that would never make it to the contract. Similar to giveTagged in #32514, I fully expected we'd be able to get this into the contract eventually.

Going to bring this discussion up in Discord, too. :)

Ultimately, nobody will ever be able to discover this method exists and probably shouldn't be relying on it being available:

image

@GrahamCampbell GrahamCampbell changed the title Add giveConfig to ContextualBindingBuilder [9.x] Add giveConfig to ContextualBindingBuilder Nov 30, 2021
@simensen
Copy link
Contributor Author

@taylorotwell taylorotwell merged commit 8d7c7a4 into laravel:master Nov 30, 2021
@taylorotwell
Copy link
Member

I doubt many people (if anyone) are writing their own implementation of this interface so will go ahead and merge it.

@simensen
Copy link
Contributor Author

@taylorotwell Thank you so very much!

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