lhttpc -> hackney. fixes #17 #21
Conversation
andreineculau
force-pushed
the
no-lhttpc-17
branch
from
d1c39d3 to
e60a62e
Compare
| %% http_request_now(Url, Method, Hdrs, Body, Timeout, []). | ||
| katt_callbacks:http_request_now(Url, Method, Hdrs, Body, Timeout, []). | ||
|
|
||
| http_request_now(Url, Method, Hdrs, Body, Timeout, []) -> |
There was a problem hiding this comment.
Why not move this to katt_util? Then you wouldn't have the problem of having to explain yourself re meck.
There was a problem hiding this comment.
Also maybe just explain that you're doing this because you want to resemble the signature of lhttpc, that has been used before. Btw, I think that fusco retains lhttpc's signature.
There was a problem hiding this comment.
re: Why not move this to katt_util?
Not a bad idea, actually: ~katt_util:external_http_request.
But the argumentation fails to hit home :) *_util shouldn't be a dump for all internal functions you want to meck.
re: lhttpc signature - unfinished PR. Obviously the signature and the tests should be cleaned up.. By who? Let me turn the wheel of fortune and I tell you right away:)
Well, the tests themselves need proper work irrespective of this PR (there must be a clean=readable=maintainable way to test katt).
re: fusco - I hope you're not arguing that one should choose fusco just because I'd keep the call signature intact :) Went with hackney in this PR due to longevity (# of commits) and popularity (# of github stars).
There was a problem hiding this comment.
Re: util - I don't see a reason why a wrapper around an http client shouldn't be in some *_util, except for when it would go to its own module. Apart from that, why "external"?
Re: fusco - we switched to it for almost any of our internal and opensource projects, it's probably not the worst. I think it can be regarded as the official successor of lhttpc. That said, nothing against using hackney either.
I could offer to clean that up if you could be more clear what has to be done besides replacing lhttpc. What's wrong with the tests?
There was a problem hiding this comment.
Re: util - I don't see a reason why a wrapper around an http client shouldn't be in some *_util,
because it doesn't have any utility outside katt.erl,.. or to put it differently - if it wasn't for lhttpc-> hackney, for meck, and then for the required module-prefixed call, would you have looked at the code and though "hmm, this http_request wrapper around lhttp:request" should probably go out of katt.erl into katt_util.erl ?
re: tests - #22 :p Nothing wrong, but it's not easy(tm) to just add a simple test. Reminds me of yaws testing :)
andreineculau
force-pushed
the
develop
branch
from
e6682f9 to
555d62a
Compare
|
ping @sstrigler - if travis is happy, are you also happy after this last commit? :) |
andreineculau
force-pushed
the
no-lhttpc-17
branch
from
b78f8cb to
3363ceb
Compare
andreineculau
force-pushed
the
no-lhttpc-17
branch
from
3363ceb to
d9f4ede
Compare
lhttpc -> hackney. fixes #17
Hackney WARNING: Supported versions of Erlang are R16B03-1, 17.3.4 and above. It is reported to work with R14B04 and R15B03-1.