Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

hubot-idobata can't response to message #1

Closed
vvakame opened this issue Feb 10, 2014 · 6 comments · Fixed by #2 or #5
Closed

hubot-idobata can't response to message #1

vvakame opened this issue Feb 10, 2014 · 6 comments · Fixed by #2 or #5

Comments

@vvakame
Copy link

vvakame commented Feb 10, 2014

https://github.com/github/hubot/blob/master/src/listener.coffee#L42
https://github.com/idobata/hubot-idobata/blob/master/src/idobata.coffee#L60

Hubot.TextMessage is not point to same function.

https://github.com/idobata/hubot-idobata/blob/master/src/idobata.coffee#L5
hubot-idobata reference to hubot-idobata's local hubot package.

adhoc patch
Hubot = require('hubot') to Hubot = require('../../../')


すみません英語苦手なので日本語で書きます。
idobataアダプタが作っているHubot.TextMessageはhubot-idobataが参照しているhubotを指すため、 https://github.com/github/hubot/blob/master/src/listener.coffee#L42 のinstanceof でfalseになります。
これは https://github.com/github/hubot/ を clone してpackage.jsonを編集して使うと発生します。

vvakame added a commit to topgate/tg-hubot that referenced this issue Feb 10, 2014
@tricknotes
Copy link
Contributor

Thanks for your report @vvakame 😄
You're right. I met the same issue on initial development.

This issue is related with the followings things:

  • The way to resolve packages by npm
  • Using instanceof by hubot

So far, I think that it is difficult to resolve this issue on hubot-idobata.
To avoid this, I always install hubot-idobata from local path:

$ npm install /path/to/hubot-idobata

Could you try this way?
If it seems good, I'll write this installation as a notice for development.


issue のご報告ありがとうございます!
実は、私も開発時にこの問題に遭遇してかなりハマりました...

どうやら、hubot での instanceof での判定と npm のパッケージ管理の仕組みの相性が悪いようです。
残念ながら今のところ、hubot-idobata 自体で解決できる方法が思いついていません。

そこで私は以下のように、 ローカルパスを指定してhubot-idobata をインストールすることにしています:

$ npm install /path/to/hubot-idobata

お手数ですが、この方法で上手くいくかご確認いただけますでしょうか...?
もし大丈夫そうなら、ひとまずの対応として README.md に開発時の注意点として記載したいと思います。

@tricknotes
Copy link
Contributor

I added development section to README.md by #2.
I think this may give warning about installation to other developers.

@vvakame
If you think this issue was not resolved, you can re-open this issue anytime.
Thanks for your reporting 🍻


ひとまず、インストールについての注意点を README.md に記載してみました #2
この解説が不十分であったりもっとよい解決方法がありそうであれば、いつでもお知らせいただければと思います!

ご報告いただき & hubot-idobata をお試しいただきありがとうございます!

@vvakame
Copy link
Author

vvakame commented Feb 25, 2014

@tricknotes Thank you for letting me know!
and sorry i was late. I'll try it! 😸


ありがとうございます!
返事が遅れてすみませんでした。少し時間ができたので、教わった方法で試してみようと思います 😸

@cynipe
Copy link

cynipe commented Apr 8, 2014

Hi, I just setup hubot-idobata as [README.md#setup](mentioned in https://github.com/idobata/hubot-idobata#setup). However, doesn't work properly instead of run command npm install /path/to/hubot-idobata - as mentioned above - So, in my opinion #2 is not for just a development case, but also normal use case.

BTW, referring to the Hipchat adapter implementation, isn't it better to specify like require('../../hubot')?


READMEのsetupにしたがって設定してみましたが、動きませんでした。
その後このIssueを参考にしてnpm install /path/to/hubot-idobataを実行したところ正常に動くようになりました。
なのでこの手順は開発用途だけではなく通常のユースケースでも必要なのではないでしょうか?

ところで、Hipchat adapterを参照してみたところrequire('../../hubot')のような形で参照しているのでこちらの方法の方がよいのではないでしょうか?(hubot, node.js周りあまり詳しくないので的はずれなことを言っているのかもしれませんが・・・)

tricknotes added a commit that referenced this issue Apr 8, 2014
`npm` loads the different hubot from the following:
* `./node_modules/hubot`
* `./node_modules/hubot-idobata/node_modules/hubot`

This cause instance mismatch using `instanceof`
* https://github.com/github/hubot/blob/v2.7.2/src/listener.coffee#L42

That's only `./node_modules/hubot` we need.
If `./node_modules/hubot-idobata/node_modules/hubot` is exist,
hubot-idobata wouldn't respond any message.

Fix #1
@tricknotes
Copy link
Contributor

Thanks @cynipe
I just found this reason thanks to your comment!

This issue is fixed by #5 and I released new version.
Could you try it?


@cynipe ありがとうございます!
まさにご指摘の通りで、おかげさまでずっと悩んでいた問題の解決のヒントとさせていただくことができました。

bot 上で試したところ require('../../hubot') はいい感じに動いてはいたのですが、hubot-idobata 単体でテストを実行させるのが難しくなってしまうため別の解決を試してみました #5
わたしの手元では直ったようなので、新しいバージョンをリリースしました。

よろしければ、お試しいただけますでしょうか…?

@cynipe
Copy link

cynipe commented Apr 8, 2014

@tricknotes LGTM 😄 👍 Thanks!!


正常に動くこと確認できました!
対応ありがとうございました。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants