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

remove optional dependencies #1173

Merged
merged 3 commits into from
Oct 16, 2014
Merged

remove optional dependencies #1173

merged 3 commits into from
Oct 16, 2014

Conversation

seanstrom
Copy link
Contributor

Fulfills #1005
Derived from #1067

This code was paraphrased from #1067 just FYI
Thanks @nemanjawork

Review @mikeal @nylen @FredKSchott @emkay

@nylen
Copy link
Member

nylen commented Oct 16, 2014

Having ENV set to the string npm install; looks a little strange:

screenshot_2014-10-15_19-08-05

Just take out the entire env: section of .travis.yml since Travis knows to run npm install by default.

@nylen
Copy link
Member

nylen commented Oct 16, 2014

Oh, and one more thing, as part of this PR you can remove the sections at the top of some of the test files that try to require optional dependencies, for example this section in test-cookies.js.

@seanstrom
Copy link
Contributor Author

@nylen updated the tests and travis.yml file

@nylen
Copy link
Member

nylen commented Oct 16, 2014

👍 thanks for taking this on. I'll merge it tomorrow if no one objects or beats me to it.

@seanstrom
Copy link
Contributor Author

YAAAYYYY!!! No problem :)

@nemanjawork
Copy link

@seanstrom You are welcome - I had no time to work out the bad merge on my pull request so you taking over and working on this is much appreciated. Thank you!

@FredKSchott
Copy link
Contributor

Woo! +1 Looks awesome

nylen added a commit that referenced this pull request Oct 16, 2014
@nylen nylen merged commit 9863b27 into request:master Oct 16, 2014
@seanstrom seanstrom deleted the support/rm-optional-dependencies branch October 16, 2014 17:38
nylen added a commit to nylen/request that referenced this pull request Oct 16, 2014
Don't allow non-require statements mixed in with module require
statements.  We can do this now that optional dependencies were removed
in request#1173.
nylen added a commit to nylen/request that referenced this pull request Oct 17, 2014
@tschaub tschaub mentioned this pull request Dec 9, 2014
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.

4 participants