[10.x] Adding Pipeline Facade #46271
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
byt3sage
changed the title
|
I've proposed a helper (#46198) but haven't heard back from the team that approves PRs. |
|
Ah! Would be good to link these PRs then!I’ll update the caching of the facade later this evening!On 25 Feb 2023, at 17:32, Jordan Dalton ***@***.***> wrote:
I've proposed a helper (#46198) but haven't heard back from the team that approves PRs.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
So I've disabled caching on the facade (I totally didn't even know what was a feature!) - happy to take suggestions from anyone else as to what needs doing to get this merged :) |
byt3sage
requested review from
rodrigopedra and
rojtjo
and removed request for
rodrigopedra and
rojtjo
|
You said you will be doing some "light tweaking". I can't review the PR until it is totally complete. Please mark as ready for review when that is done. |
|
Hey, sorry - it’s ready for review nowSent from my iPhoneOn 2 Mar 2023, at 19:10, Taylor Otwell ***@***.***> wrote:
You said you will be doing some "light tweaking". I can't review the PR until it is totally complete. Please mark as ready for review when that is done.
—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
Just to re-iterate, the light tweaking was just making sure the correct docblocks etc were added - hence I requested a re-review from the people who raised additional comments :) |
This was referenced Mar 13, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hey All!
I have a feeling there is a distinct reasoning behind why this hasn't been done before but here it goes. I love pipelines... I also love facades... Why not merge the two and have a pipeline facade?
This is the simplest implementation I could think of. It almost feels too simple so I'm sure there's some form of logic I have missed.
I'd love some feedback to try get this pushed into Laravel as a core facade!
Happy for this to be closed if it's a non-starter!
P.S - I'll be doing some light tweaking of bits, just thought I'd push this up first so I don't waste my time if it's never going to be accepted :)