search

austinkelleher / giphy-api

🎥 Isomorphic JavaScript client for the Giphy.com API
MIT License
86 stars 19 forks source link

Breaking change in minor version #41

Closed TimvdLippe closed 5 years ago

TimvdLippe commented 6 years ago

In https://github.com/austinkelleher/giphy-api/commit/177af34305d3b94dd2e4f50c35ab374e0e44757a#diff-168726dbe96b3ce427e7fedce31bb0bcL63 you removed the assignment to httpService on the api object. This is a breaking change, as users (in this case me) rely on this field. Could you add the assignment back?

austinkelleher commented 6 years ago

Hey, @TimvdLippe. I apologize for the breaking change and not responding quickly. I overlooked this issue. Is there a reason why you were using the httpService field? This was intended to be a private implementation detail. It should have probably been prefixed with an underscore.

TimvdLippe commented 6 years ago

I am not the original author of the package where I encountered this usage. In essence, httpService was used in the tests to verify that the correct calls were made by the downstream project: https://github.com/patsissons/hubot-giphy/blob/be50b92a92f2104468244845b8a57d79ec0173eb/test/giphy.spec.js#L1268

TimvdLippe commented 5 years ago

Oh awesome. Thanks @austinkelleher for fixing this :tada:

austinkelleher commented 5 years ago

@TimvdLippe So sorry for the late response. I totally forgot to revisit it. I am going to open a PR soon to drop support for old Node versions and then I'll open a PR on hubot-giphy with the new version. Thanks again for opening the issue.

TimvdLippe commented 5 years ago

I was about to open a PR on hubot-giphy, but please go ahead and do so. Thanks again for your effort and getting back to this, much appreciated!