Update to work with latest request under node

[KrzysztofMadejski]

Commenting on:
// we could just call request but that's a PITA to mock plus request.get = request (if you look at the source code)

This is not anymore true, method=GET is forced when using get(): https://github.com/request/request/blob/master/index.js#L60

[rufuspollock]

Thanks for the patch. One quick question: do we need to now set an explicit >= version for request in package.json dependencies?

[KrzysztofMadejski]

Browsing history it was changed in commit request/request@39396b0#diff-168726dbe96b3ce427e7fedce31bb0bc

Which is v2.35.0, but the code I posted should work with previous versions as well, because request == request.get in old versions.

Coming back to your question: In fact you should set explicitly version in package.json, so the request will not be bumped to v3 (backwards-incompatible probably) in future (see: http://blog.nodejitsu.com/package-dependencies-done-right/)

So: "request": "2.x" or "request": "2.x.x" (not sure which one works and how request is handling backwards compatibility) would be probably the best. If you want to be super safe test it on latest and put 2.51.x as patches are expected not to break functionality.

P.S. Underscore dependency should be treated the same way.

[rufuspollock]

@KrzysztofMadejski I'd welcome you making those suggested changes re explicit dependencies. Then we can merge the patch :-)

[KrzysztofMadejski]

Sure, I wanted though to run tests beforehands and I'm not sure how to do it. There's no documention on it.

Opening test/index.html throws a lot of 404 errors ie. http://localhost/src/recline/test/qunit/qunit.css Failed to load resource: the server responded with a status of 404 (Not Found).

[rufuspollock]

Hmmm, good point. I realize now we haven't actually done the port of tests to mocha or similar to allow nodejs tests - see #17

Given straightforward nature of this change we can merge the package.json changes I think - though if you wanted to have a go at #17 that would be amazing!

[KrzysztofMadejski]

I'll think about #17 :)

Meanwhile I've set dependencies, "tested" it in my own scenario (updating CKAN via Action API v3) and bumped patch version. It's ready to merge!