Skip to content

[8.x] Make more Builder/Grammar methods public #34102

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 3 commits into from
Sep 3, 2020

Conversation

mfn
Copy link
Contributor

@mfn mfn commented Sep 3, 2020

  • Make Builder::cleanBindings public
  • Make Grammar::compileWheres public

This is similar in spirit to #33953 (which ultimately was only applied to 8.x via #33984 )

I've use cases where I want to be able to re-use Builder/Grammar without having to extend them, as they might already be extended from other packages and thus makes compositing via replacing very hard.

By having those method public, they can be easier re-used from the outside. Some rational of this is explained in an issue over at staudenmeir/laravel-upsert#21 (comment) about issues I faced using a package providing upsert, and using a different approach to implement this.

Thanks!

themsaid and others added 3 commits September 2, 2020 15:01
I've a use case where I want to composite functionality which can take
advantage of the existing code here, without e.g. extending the
Query Builder
I've a use case where I want to composite functionality which can take
advantage of the existing code here, without e.g. extending the Grammar
@GrahamCampbell GrahamCampbell changed the base branch from master to 8.x September 3, 2020 09:57
@taylorotwell taylorotwell merged commit 3b48dee into laravel:8.x Sep 3, 2020
@mfn mfn deleted the mfn-method-public branch September 3, 2020 13:38
@mfn
Copy link
Contributor Author

mfn commented Sep 3, 2020

Thanks!

Sorry for the wrong branch, didn't realize 8.x already existed. Go Team Laravel!

@GrahamCampbell
Copy link
Member

Yeh, we usually cut a new branch around 2-3 weeks before release.

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.

4 participants