search

observing / fullcontact

FullContact API bindings for Node.js
MIT License
42 stars 18 forks source link

Webhook support for person.email #6

Closed AdriVanHoudt closed 9 years ago

AdriVanHoudt commented 9 years ago

Added webhook support for person.email lookup and added a test for it. Also added a test for the empty phone test :wink:

3rd-Eden commented 9 years ago

LGTM except for the removal of 0.8

AdriVanHoudt commented 9 years ago

If you want I can enable it again?

3rd-Eden commented 9 years ago

@AdriVanHoudt I'll add it back, thanks for the pull request. I'll kick out a new minor version in a bit.

AdriVanHoudt commented 9 years ago

Thanks! and I just pushed the 0.8 support to my fork :smile:

3rd-Eden commented 9 years ago

0.8 seems to fail, i'm gonna nuke it

AdriVanHoudt commented 9 years ago

I got Error: usage limits are exceeded maybe the key used was used to much

3rd-Eden commented 9 years ago

@AdriVanHoudt Yes, nothing I can do about that unfortunately. Full contact doesn't have any keys that can be used for testing and I didn't really feel like mocking every single response up.

AdriVanHoudt commented 9 years ago

Yeah no prob, I know that some libs require the dev to use his own key, maybe that will help?

3rd-Eden commented 9 years ago

@AdriVanHoudt I could surely do that, but that requires extra magic to make the tests work with Travis-CI. (Secret env variables etc) and you would still be limited to the API key.

AdriVanHoudt commented 9 years ago

@3rd-Eden true maybe advice devs in the readme to use their own keys with local testing?

3rd-Eden commented 9 years ago

@AdriVanHoudt Moved the API key, added a dedicated test script for travis-ci so that should help prevent more failures in the future. And i'll update the docs in a bit.

AdriVanHoudt commented 9 years ago

@3rd-Eden cool! If I have time i'll look into adding webhook support for all the other endpoints to(where possible)

3rd-Eden commented 9 years ago

@AdriVanHoudt I'm also more than happy to add you as contributor to the module if you want to. It would probably allow you to iterate a bit quicker. (As long as you don't break the existing tests ;-))

AdriVanHoudt commented 9 years ago

@3rd-Eden fine by me :smile: tbh I'm pretty new to the open source stuff but I'll do my best! (to not break everything :laughing: )

3rd-Eden commented 9 years ago

E-mail send, if you need npm access as well, give me a poke.

AdriVanHoudt commented 9 years ago

I saw it thanks! Maybe for now not so you are required to review changes :wink: