clearbit / clearbit-node

Node library for querying the Clearbit business intelligence APIs
https://clearbit.com/docs
MIT License
69 stars 35 forks source link

Use arrow functions instead of .bind(this) #33

Closed papandreou closed 6 years ago

papandreou commented 6 years ago

See #32

papandreou commented 6 years ago

Ah, I didn't notice that the module is being tested with node.js 0.10 on Travis until now :). I updated the branch to use a var that = this approach instead of arrow functions, in case 0.10 must still be supported :)

papandreou commented 6 years ago

Haven't observed #32 in our app after deploying this fix to production 3 days ago, so I think the fix is good :)

holm commented 6 years ago

Any chance to get a review of this?

papandreou commented 6 years ago

@davidlumley, ping?

maccman commented 6 years ago

Sorry, I don't think we will merge this. What's the issue with bind?

davidlumley commented 6 years ago

Sorry to hear NewRelic is breaking some things. I'm on vacation at the moment, but happy to discuss with @maccman when I get back.

papandreou commented 6 years ago

I've dug around some more and reported it to the newrelic folks, who have come up with a fix: https://github.com/newrelic/node-newrelic/pull/260#issuecomment-369089224

Thanks for taking the time to consider the issue, happy that it won't be necessary after all :)