Hubot adapter convention broken

[maxgoedjen]

I see every adapter including instructions to include hubot as a dependency in the hubot you're installing the adapter into. This is weird and broken. Doing this leads to issue #425, specifically the issue I detailed in this post.

If I import the nice way, like this: {Robot, Adapter, Response, TextMessage, EnterMessage, LeaveMessage} = require 'hubot' Hubot won't respond.

However, if I do something gross instead like {TextMessage, EnterMessage, LeaveMessage} = require '../../../src/message', Hubot works and responds perfectly fine.

Can you clarify whether this is a bug in Hubot or whether basically every extension is doing it wrong?

[3100]

I'm having this same issue in my adapter. I use process.env like below for quick fix:

{Robot, Adapter, TextMessage, Response} = require process.env.HUBOT_XXX

I think this is not good.

[maxgoedjen]

@3100 well, it's probably slightly better than my suggested hacky fix :P

[iangreenleaf]

I had trouble with this in the past (#332), but recently it seems like something resolved the issue. What version of hubot and npm are you running?

[maxgoedjen]

@iangreenleaf I'm at lastest of both, master on hubot and, 1.2.11 on node.js as of writing. I bet you're right. I'm not actually a node.js developer, really, but it seems like there should be a better way of doing this in general.

[pyro2927]

hubot-gtalk lists hubot v2.4.5 as a dependency, but using your require trick above with v2.4.5 does't work. Using it with v2.4.7 works, but you're right, it's an ugly workaround. This is definitely a bug with hubot.

[tombell]

Adapters shouldn't have hubot as a dependency. This is because hubot loads adapters therefore is already present.

[technicalpickles]

They probably need it as a development dependency though, for testing and whatnot.

This thread makes me think that the the campfire adapter should be extracted out of hubot (but still maintained by), in order to help define the best practices for adapters. As it is, with the adapters part of hubot, the campfire & shell adapters don't run into the same issues that external ones do.

On Mar 5, 2013, at 10:49 AM, Tom Bell notifications@github.com wrote:

Adapters shouldn't have hubot as a dependency.

— Reply to this email directly or view it on GitHub.

[tombell]

Then it should be listed under devDependencies in the package.json not dependencies. There are plenty of working external adapters out there, people will write buggy ones and not update them often, not much we can do about it.

[tombell]

To clarify, the adapter is doing it wrong if they're listing hubot as a dependency not a devDependency in the package.json.

[drdamour]

i've also come to this conclusion, trying to work with the talker adapter. I found if i ran bin/hubot from the node_modules/hubot directory the relative path loading works again because the running hubot is the referenced hubot (most importantly only one TextMessage class is loaded).

of course if you do this, the path logic for hubot-scripts breaks down.

I support the idea of moving campfire to it's own package as a reference implementation of adapters. If anyone knows of a different reference implementation can you share?

[drdamour]

@tombell i believe your suggestion doesn't apply, the problem is the instructions tell you to put it as a dependancy in the hubot package.json. efefctively making your local hubot depend on the packaged hubot.

[tombell]

What instructions?

[drdamour]

for example the talker adapter https://github.com/github/hubot/wiki/Adapter:-Talkerapp & hipchat https://github.com/github/hubot/wiki/Adapter:-HipChat & flowdock https://github.com/github/hubot/wiki/Adapter:-Flowdock

really any of them as the original post says

[tombell]

Yes, hubot is a library to run a hubot, you need to use create a hubot using hubot -c. You do not run hubot from a clone of this repo.

[drdamour]

i haven't seen any instructions that indicate that. IE: https://github.com/github/hubot/wiki/Deploying-Hubot-onto-UNIX tells you to run the /opt/hubot/bin/hubot executable Is a step missing from that wiki?

also, the way hubot-scripts is referenced (using node_modules) seems to indicate that you run the local copy of the repo.

i'll give -c a shot, but i'll be surprised on what it can do to fix this

[tombell]

The deploying to UNIX aren't the official instructions, someone else wrote those. The instructions we support are https://github.com/github/hubot/wiki/Deploying-Hubot-onto-Heroku, you can use those for a local hubot by skipping the Heroku configuration and pushing.

[drdamour]

@tombell you seem to be pushing me all around without trying to solve the issue. The directions you link DO NOT include any information about a -c command. can you please take some time to evaluate the issue yourself before replying with some documentation that doesn't mesh with what you are saying, it's very difficult to work this way.

[tombell]

People have been messing with the wiki entries and they're now completely full of incorrect information. Here is an example of a hubot 'instance' that you run https://github.com/tombell/a-hubot you can add the adapter to the package.json then run with bin/hubot -a <adapter>.

There isn't a problem with hubot, there is a problem with people editing the wiki with incorrect information without confirming with a collaborator of the project.

[drdamour]

i agree the docs are messed up, and since you know that i'd ask that you peek at them before referencing them as a solution. -C does seem to be taking me down the right path.

I'll update the unix docs once i have success.

question, why on a -C switch isn't the bin/hubot script marked as executable?

[tombell]

The code doesn't set the executable property on the bin/hubot script after it copies them.

If you clone this repo and run make package it'll create a hubot with -c and mark the script as executable.

[tombell]

I am planning on getting the wiki removed and having official documentation in the repo that not just anyone can edit.

[drdamour]

so a rough workflow of what you're saying looks something like this:

1. download the source
2. run make package (which runs hubot -C hubot)
3. put the newly created hubot directory somewhere, like /opt/hubot
4. Add any customized dependancies for adapters/external scripts to the /opt/hubot/package.json file
5. Store your /opt/hubot/package.json file in some repo
6. run /opt/hubot/bin/hubot (this installs all the dependancies including hubot itself, and runs hubot)

how do you update hubot in the future? does the npm install in /bin/hubot do an implicit update, or do you suggest running "npm update" explicitly as needed?

[iangreenleaf]

This thread makes me think that the the campfire adapter should be extracted out of hubot (but still maintained by), in order to help define the best practices for adapters. As it is, with the adapters part of hubot, the campfire & shell adapters don't run into the same issues that external ones do.

:+1: :+1: :+1:

It would be really nice to have a model adapter implementation for this stuff. The current process is:

1. Read the (fairly incomplete) official docs.
2. Read whatever 3rd-party adapters you can find.
3. Guess.

[tombell]

Yeah that looks correct. You can git init in the /opt/hubot and keep that as a repo.

To update hubot when a new version is pushed to npm you just need to edit the hubot and/or hubot-scripts versions in the package.json and then run npm install which I think the bin/hubot already runs.

For example I if v2.4.8 came out you would update this line https://github.com/tombell/a-hubot/blob/master/package.json#L18 and run bin/hubot.

[drdamour]

indeed, i've run through that workflow and i no longer get the broken behaviour of the adapter.

@maxgoedjen were you running locally on Nix, or on heroku?

[maxgoedjen]

@tombell so basically ou have to deploy it to have adapters work? That basically makes it pretty messy to write adapters. Is there a (slightly) nicer way to do it?

@drdamour I'm seeing this behavior on OS X (dev) and Ubuntu (prod).

[drdamour]

@maxgoedjen you don't have to deploy to heroku if that's your meaning of deploy, but to run hubot it seems you need to run "make package" and install the output of that.

how does that make it messy to write adapters?

[maxgoedjen]

@technicalpickles @iangreenleaf I would love it if they were moved into an adapters repo to show best practices.

[maxgoedjen]

@drdamour it just seems weird that the "best practice" of having hubot in devDependencies lends itself to adapters that wouldn't work, correct? Seems odd.

[drdamour]

@maxgoedjen devDependencies was not the recommendation, see https://github.com/github/hubot/issues/428#issuecomment-14452760

[maxgoedjen]

@drdamour it just seems like I should be able to run bin/hubot -a ADAPTER and test it just like I would the campfire adapter. Basically what you're saying is I have to make a release build every time I want to test a change?

[drdamour]

@maxgoedjen you're not supposed to be doing what you propose. Yes it works, but not fully. For example the hubot-scripts package is not included and none of the scripts from that package will load. Also the external scripts will fail to work without hand modification.

think of it this way, any time you download source you usually have to compile/make that into the release package. i'm not a node expert though, is it common for you to be able to local clone a node package repo and just run it and expect it to work?

[maxgoedjen]

@drdamour is that intended though? it sounds like it used to work and recently broke or something.

On the node thing, this is the first time I've worked in node, so I can't say with any authority.

[drdamour]

@maxgoedjen per this thread that is the intended use, the fact that you could just run the local copy of this repo was a side effect of the package organization.

this is even described (somewhat cryptically) @ https://github.com/github/hubot#getting-your-own

[maxgoedjen]

@drdamour fair enough. Really needs to be documented somewhere though, because almost every single adapter does it wrong.

[drdamour]

@maxgoedjen i wouldn't say they do it wrong, but they definitely lead you down an ambiguous path. they created https://github.com/github/hubot/issues/429 to address the documentation problem.

[maxgoedjen]

@drdamour almost every repo includes instructions to have hubot have a dependency of itself... That's wrong, isn't it?

[drdamour]

@maxgoedjen no it's not. The way this thing is setup is you have a node app that depends on the hubot library. That node app is defined in the template directory. when you run "make package" in this repo's root it creates that node app. that app is named "hosted-hubot". it's dependancies by default are:

"dependencies": { "hubot": ">=2.4.7", "hubot-scripts": ">= 2.4.1", "optparse": "1.0.3" },

see, hubot is already there!

every adapter when they say modify the package.json file, they are talking about the hosted-hubot package.json file, NOT the package.json file for the hubot library. so they aren't wrong, it's just darn confusing!

[tombell]

I am going to work on improving the documentation that will NOT be publicly editable and will be part of the repo.

I will say it for the last time once and for all: YOU DO NOT RUN HUBOT FROM THIS REPO AS YOUR HUBOT. This is why a lot of these issues are caused. People doing it this way is what caused them to edit the docs as stuff didn't work, so the docs became incorrect for people doing it the correct.

Your 'hubot' will be the output of running bin/hubot -c <output dir> (or make package which will make a hubot directory in the same directory). See example here: https://github.com/tombell/a-hubot. You can move this directory to another location and/or commit it to a git repository. The hubot npm package is a library for creating/running a hubot, not BEING an instance of a hubot.

When you follow the above instructions for 'creating a hubot' the documentation for adapters makes sense. From now on if a person opens an issue where by are running hubot from this repo, and are having issues they will be told to setup hubot correctly and the issue will be closed.

If you don't feel comfortable with the above instructions you can clone/fork https://github.com/tombell/a-hubot and use that as a base.

[maxgoedjen]

@drdamour ah. Confusing indeed. Closing this out as it was never supposed to work and documentation is being addressed in #429

[drdamour]

@tombell have you considered making hosted-hubot it's own repo?

[tombell]

Once the documentation is properly up to date it probably won't be worth it. I will leave my repo up until the documentation is complete as a reference point.

[drdamour]

@tombell if this is a normal structure, for a node repo to include the library and the app in the same repo, then i guess documentation is enough, but if this is abnormal no amount of documentation will eliminate this problem. Perhaps there's a way to poison pill running hubot from the repo.

additional thought: if hosted-hubot was it's own repo, then the process to get your own could include forking that project.

[tombell]

hubot is only a library, not both. We used to offer a download of the 'hosted-hubot' but GitHub removed downloads on repos. This is probably what confused a lot of people and the docs were incorrectly updated.

[drdamour]

from my outsider new user perspective it's a library that has an executable to generate an app that references said library. thus it has the app & the library. It's circular. circular is confusing.