Skip to content

Guzzle proxy support#29

Open
holtkamp wants to merge 12 commits into
link0:masterfrom
holtkamp:guzzle-proxy-support
Open

Guzzle proxy support#29
holtkamp wants to merge 12 commits into
link0:masterfrom
holtkamp:guzzle-proxy-support

Conversation

@holtkamp

Copy link
Copy Markdown
Contributor

As suggested in #27

@dennisdegreef

Copy link
Copy Markdown
Contributor

This looks awesome! Thanks for this!

One question though, why are docblocks being removed in this PR?

@holtkamp

Copy link
Copy Markdown
Contributor Author

One question though, why are docblocks being removed in this PR?

mmm, in my opinion they should only be used when adding information... Since PHP 7.1 proper return types are supported, so no PHPDoc is required anymore...

Also see this thread: https://twitter.com/akrabat/status/878345065696985088

@dennisdegreef

Copy link
Copy Markdown
Contributor

Interesting. I tend to agree with the provided point.
On the other hand, should we then allow multiple types for a proxy? Namely, a string or an array? Or should we choose one over the other?

@holtkamp

Copy link
Copy Markdown
Contributor Author

On the other hand, should we then allow multiple types for a proxy? Namely, a string or an array? Or should we choose one over the other?

First I went with array, since it provides more options for configuration. But eventually, to be able to use a static IP, a plain HTTP proxy suffices => so I went with the string.

Then probably only the string suffices.... But hey, why limit it? The current approach allows for best of both worlds and it is up to the user what info is handed over to Guzzle...

@dennisdegreef

Copy link
Copy Markdown
Contributor

So can we typehint on an array (also allowing a single entry) and validate the strings in that array?
As long as we are pushing towards stricter typehinted code, might as well only allow a single type as a parameter.

@holtkamp

holtkamp commented Jul 2, 2017

Copy link
Copy Markdown
Contributor Author

So can we typehint on an array

Sure, but why restrict the options? We can just follow the Guzzle interface for it right? string + array
http://docs.guzzlephp.org/en/stable/request-options.html#proxy

@holtkamp

holtkamp commented Jul 9, 2017

Copy link
Copy Markdown
Contributor Author

shameless bump on this? 😄

@holtkamp

holtkamp commented Aug 1, 2017

Copy link
Copy Markdown
Contributor Author

@dennisdegreef another bump on this?

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.

2 participants