Skip to content
This repository was archived by the owner on Nov 14, 2019. It is now read-only.

Added support for using proxy servers #132

Closed
wants to merge 2 commits into from

Conversation

javiereguiluz
Copy link
Member

I've tested it successfully with some free public proxy servers, but we should definitely test it with "real" proxy servers.

*
* @return Client
*/
protected function getGuzzleClient()
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason not to make this private?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can't call private methods in subclasses :)

Copy link
Member

Choose a reason for hiding this comment

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

It wasn't used in another class the time I wrote the comment. :D

Copy link
Member Author

Choose a reason for hiding this comment

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

You both are right! Thanks for reviewing the code.

@xabbuh
Copy link
Member

xabbuh commented Apr 1, 2015

The NewCommand class also uses the Guzzle client to download the list of available versions.

@javiereguiluz
Copy link
Member Author

Nice catch! Fixed. Thanks.

@javiereguiluz
Copy link
Member Author

I'm going to merge this now and release a new installer version. The code looks "safe enough" to not break anything if you are not using a proxy. And it also looks like we're going to solve the problem at least for some proxy users.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants