athieriot / hubot-yammer

This is the Yammer adapter for hubot that allows you to send a message to him with Yammer and he will happily reply the same way.
31 stars 26 forks source link

Removing unneeded dependencies #25

Closed emedvedev closed 8 years ago

emedvedev commented 8 years ago
  1. hubot-yammer doesn’t “depend” on hubot in the package.json sense, it’s the other way around. :) It’s a dev-dependency at best (hubot-slack lists it in dev, hubot-hipchat doesn’t list it at all), and installing inside a docker container with a hubot dependency is suboptimal because it’ll fetch hubot the second time.
  2. I couldn’t find any reference to request in the code, so I’m removing it. @athieriot: need your input here, you might know something I don’t. :)
athieriot commented 8 years ago

1) You are right. If you feel better that way you can remove hubot depdency and add it later if it's need for unit tests 2) It use to be used by the yammer library included in the source (Reference: https://github.com/athieriot/hubot-yammer/commit/738b72649927c047be2e873891d5bf423ea09594#diff-25d80a6e8a40393e836b15f145a2ff8dL1) 3) I now there is no 3 :p But what would you think of cleaning up the main property of the package.json and upgrading engine version of Node.js (That's up to you though, not sure if it makes any difference)?

If you could just squash those two commits together I'll approve it.

P.S: Sorry for the delay ! We are not quite in the same timezone (I'm on Europe/London so your morning should be my afternoon) :p

emedvedev commented 8 years ago

Hm, yeah, I'll clean it up and see if I can upgrade Node. Will get back to you on that. Don't worry about the delay :)

emedvedev commented 8 years ago

Checked the comment out, I don't think we can remove 'main' for coffeescript at all, and node version is fine. Up to you if you want to change anything else while we're at it. :) LGTM

athieriot commented 8 years ago

Up to you :) Was just a comment. If you're fine with that.

I would definitively squash your two commits together though

emedvedev commented 8 years ago

Done. LGTM

athieriot commented 8 years ago

LGTM