search

hedwig-im / hedwig

An Adapter-based Bot Framework for Elixir Applications
MIT License
650 stars 73 forks source link

Check for empty string robot names #46

Closed enilsen16 closed 7 years ago

enilsen16 commented 8 years ago

I have been looking into using Hedwig (see example) for a messenger bot and didn't want to have to prefix messages with a robots name. It seems that an empty string is a valid name (hopefully by design). Anyway i think it might make sense to add a note to the readme about this. If not the readme then maybe the generator itself?

coveralls commented 8 years ago

Coverage Status

Coverage remained the same at 87.006% when pulling adcc67a19aa37cb31d163681a10a4ae22504ca47 on enilsen16:readme into 090048c58d0831c4d3b97e15f592c6af8e4a6c02 on hedwig-im:master.

scrogson commented 8 years ago

Hey there @enilsen16!

I think that is a bug and not by design. If you don't want to mention the bot's name for a responder to match, you should use the hear macro. respond is for matching when the bot is mentioned.

Maybe the docs could be more clear about hear vs. respond?

enilsen16 commented 8 years ago

oh interesting, after looking closer at the example responders that makes sense. Let me go ahead and add a check in the generator then.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.9%) to 85.083% when pulling b60c802f59b3b963eda9d3542e77d517710bcc0c on enilsen16:readme into 090048c58d0831c4d3b97e15f592c6af8e4a6c02 on hedwig-im:master.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-1.4%) to 85.561% when pulling 57b1d2b177acf41c40eec8c0a8b163d8f1a98615 on enilsen16:readme into 090048c58d0831c4d3b97e15f592c6af8e4a6c02 on hedwig-im:master.

enilsen16 commented 8 years ago

Hey @scrogson,

I went ahead and added some checks in the generator and in the supervisor to prevent the same mistake I made.

enilsen16 commented 8 years ago

@scrogson Let me know if something is missing here... 👍