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

Add tests and docs for using the agent, agentClass and agentOptions. Forever option defaults to using http(s).Agent in node 0.12+ #1671

Merged
merged 2 commits into from Jul 20, 2015

Conversation

simov
Copy link
Member

@simov simov commented Jul 10, 2015

Fixes #1637
Fixes #1669

Feedback: @mikeal @devtristan @triccardi-systran and anyone else interested in

  • Take a look at the agent tests here. Just a bunch of simple use cases, also note that the forever option defaults to http(s).Agent in node 0.12+
  • Take a look at the docs as well. A little bit down below there is a section for agent related options. It would be great if you read through all of them carefully and let me know if I'm not getting something right.

Forever option defaults to using http(s).Agent in node 0.12+
@simov
Copy link
Member Author

simov commented Jul 16, 2015

@mikeal @devtristan @triccardi-systran (+ anyone else) any feedback on this one?

@summer4096
Copy link
Contributor

I don't love forever being 0.10 only, as the docs say, but the code clearly supports the option in more recent versions. Why not get rid of the console.warn and keep forever as an option available for all versions?

@simov
Copy link
Member Author

simov commented Jul 16, 2015

Probably a more appropriate way to document the forever option would be to say that it defaults to http.Agent in node 0.12+ which was suggested by @mikeal, also that's the way to get rid of potential bugs such as the memory leak one in your (@devtristan) issue here #1637. The console.warn can be removed in that case I agree.

@simov
Copy link
Member Author

simov commented Jul 17, 2015

@devtristan updated the docs and removed the warning.

simov added a commit that referenced this pull request Jul 20, 2015
Add tests and docs for using the agent, agentClass, agentOptions and forever options.
Forever option defaults to using http(s).Agent in node 0.12+
@simov simov merged commit a2aea33 into request:master Jul 20, 2015
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