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

Feature | V2 Better Merging Pipeline #144

Merged
merged 4 commits into from
Jan 24, 2023

Conversation

Sammyjo20
Copy link
Member

@Sammyjo20 Sammyjo20 commented Jan 24, 2023

This PR makes some more improvements to the middleware pipelines, specifically switching to a Pipe class for each pipe which now contains the name. This means I no longer need a separate array for the pipe names and when merging pipelines, all the names can be brought across.

The default fixture directory will now be created even when it doesn't exist. @simple-hacker @Gummibeer

Todo: Throw an exception if merging results in two of the same middleware?

@what-the-diff
Copy link

what-the-diff bot commented Jan 24, 2023

  • Added a new interface for the pipeline
  • Removed getRawResponse from Response contract as it was not being used anywhere in Saloon or any of its packages
  • Created Pipe class to hold pipe data and added type hints where possible
  • Changed Pipeline constructor to accept an array of pipes instead of individual arguments, this is because we are now using named pipes which can be passed into the constructor via an array (this also means that you cannot pass multiple unnamed pipes)
  • Fixed some typos in comments/docblocks etc...

Copy link
Member Author

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Self review

@Sammyjo20 Sammyjo20 marked this pull request as ready for review January 24, 2023 22:26
src/Helpers/Pipeline.php Outdated Show resolved Hide resolved
Copy link
Member Author

@Sammyjo20 Sammyjo20 left a comment

Choose a reason for hiding this comment

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

Self review

@Sammyjo20 Sammyjo20 merged commit bc3909e into v2 Jan 24, 2023
@Sammyjo20 Sammyjo20 deleted the feature/v2-better-merging-pipeline branch January 25, 2023 23:32
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