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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Support For Multiple Rooms #29

Merged

Conversation

leefaus
Copy link

@leefaus leefaus commented Aug 27, 2015

@geekgonecrazy @Sing-Li

Can I get some 馃憖 on these changes?

- added simplified setup through npm link (docs needed)
- added support for multiple rooms
@geekgonecrazy
Copy link
Member

Looks good. Some reason doesn't work through docker.

Couple of little things:
ROCKETCHAT_ROOM probably should just have GENERAL in it. Since that other rooms id will be different on other servers.
ROCKETCHAT_URL probably should be localhost by default

@Sing-Li
Copy link
Member

Sing-Li commented Aug 27, 2015

Just tried it here, in my 'lab environment', on docker as well.

WORKED like a charm. @geekgonecrazy - please merge it down once you confirmed. Tested for general + one other room.

TWO gotchas, nothing to do with the code, I encountered:

  • forgot to pull down new docker image - docker pull singli/hubot-rocketchat
  • forgot to do npm install after git clone of lee's branch

After that, I just edited the ROCKETCHAT_URL, the ROCKETCHAT_ROOM list, user and password; I also tested @geekgonecrazy 's BOT_NAME change by using -e "BOT_NAME=yeahbaby" and it worked fine.

@geekgonecrazy
Copy link
Member

Had to purge node_modules and do npm install again. Now works.

Maybe we can include some of this in the image in the future

geekgonecrazy added a commit that referenced this pull request Aug 28, 2015
@geekgonecrazy geekgonecrazy merged commit fb96cb9 into RocketChat:master Aug 28, 2015
@leefaus
Copy link
Author

leefaus commented Aug 28, 2015

@geekgonecrazy

I have some notes I need to add to the Hubot documentation around how to build without requiring a Hubot dependency. That will make it easier for deployments too.

@geekgonecrazy
Copy link
Member

@leefaus sounds good. No fun writing documentation, but sure is helpful 馃憤

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

Successfully merging this pull request may close these issues.

None yet

3 participants