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

Expose the internal middleware queue in MiddlewarePipe as an iterable #57

Merged
merged 4 commits into from
Jul 17, 2024

Conversation

gsteel
Copy link
Member

@gsteel gsteel commented Jul 17, 2024

Q A
BC Break yes
New Feature yes

Description

Returns the internal SplQueue as an array so that users can inspect the pipeline.

Closes #44

Returns the internal `SplQueue` as an array so that users can inspect the pipeline.

Signed-off-by: George Steel <[email protected]>
@gsteel gsteel force-pushed the v4/middleware-pipe-to-array branch from a80f851 to 8f7a08c Compare July 17, 2024 09:17
Comment on lines 14 to 15
/** @return list<MiddlewareInterface> */
public function toArray(): array;
Copy link
Member

Choose a reason for hiding this comment

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

IMO not to be interfaced 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

I thought so too… If you need the convenience of inspecting the pipeline, like in tests, then it's not much to ask to first assert the implementation type.

Copy link
Member

Choose a reason for hiding this comment

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

@gsteel what about @Xerkus' suggestion in #44? Or even yours?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've added a new interface with no additional methods that extends from MiddlewarePipeInterface and IteratorAggregate so that users can test for IterableMiddlewarePipeInterface and simply call iterator_to_array on the pipeline instance. Better?

@gsteel gsteel changed the title Add toArray to MiddlewarePipeInterface Add toArray to MiddlewarePipe Jul 17, 2024
…g the pipeline itself iterable

Signed-off-by: George Steel <[email protected]>
@gsteel gsteel changed the title Add toArray to MiddlewarePipe Expose the internal middleware queue in MiddlewarePipe as an iterable Jul 17, 2024
Copy link
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines 10 to 11
/** @extends IteratorAggregate<int, MiddlewareInterface> */
interface IterableMiddlewarePipeInterface extends MiddlewarePipeInterface, IteratorAggregate
Copy link
Member

Choose a reason for hiding this comment

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

Since this is not internal, I would describe it: other than that, all good!

@Ocramius Ocramius self-assigned this Jul 17, 2024
@Ocramius Ocramius merged commit 176cf8d into laminas:4.0.x Jul 17, 2024
11 checks passed
@gsteel gsteel deleted the v4/middleware-pipe-to-array branch July 17, 2024 12:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants