Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

two minor patches #1

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
2 participants

added support for HTTP_PROXY (can be configure in the configure block) and removed a non-exist file from Manifest file.

I'm not sure I follow why the specs have been bifurcated into 'with proxy' and 'without proxy'. It seems to me that all we need to do is just write a spec that guarantees the new ExactTarget#create_http method returns the correct results given each configuration. In the rest of the spec we're fine just using the mocked up Net::HTTP.

Also, the ExactTarget#create_http method can be a little dryer but returning the class (either the proxy class or Net::HTTP), and then the calling code would be create_http.new(uri.host, uri.port) -- might even make more sense to name that method ExactTarget#net_http_or_proxy.

Looks good. Thanks.

Contributor

david-mccullars commented May 13, 2011

I have implemented dd5b108 slightly differently but with the same end result. Both 8961f84 and 0d14b29 have been cherry-picked in. Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment