Cannot set custom adapter path through constructor of Robot class

[rqc]

Hi,

I think there is an error here: https://github.com/github/hubot/blob/498d65611388d2881e6ae24f74bd68a832a24211/src/robot.coffee#L43-L44

Since in the constructor argument for adapterPath is expected to be nil or existing, @adapterPath will never be existing since it is not declared anywhere (the constructor uses adapterPath and not @adapterPath).

[mose]

You are totally right. But I suspect nobody noticed because in the current state there is no way to set a custom adapterPath without some hacking in hubot launch script. I suspect it may have been put there for some yet-to-come evolution of the code.

[technicalpickles]

#1191 is a similar question.

I suspect it may have been put there for some yet-to-come evolution of the code.

It was introduced in #1109 which is work towards #858

[technicalpickles]

Looked a little more closely, and I think it was just an oversight. It'd make sense to do @adapterPath = adapterPath or Path.join __dirname, "adapters"

Some part of me suspects I thought it was using the coffeescriptism of having constructor args like constructor: (@adapterPath), which would automatically set @adapterPath for you.

[rqc]

@mose @technicalpickles thanks for looking into this. Would you like me to issue a PR?

[mose]

Sure, go for it, that should be a simple one :) Thanks!

[technicalpickles]

I am thinking a bit more about this as I worked on #1366 . According to the docs, it's a String of the path to built-in adapters (defaults to src/adapters). I'm not sure it makes sense to be able to override where builtin adapters are loaded from at a high level, mostly because I would expect making other adapters as new packages. There aren't any public hubot APIs that you can use to override this, so I think that would be pretty unsupported. Even if the adapterPath was set, it would only load the hard-coded set of adapters (HUBOT_DEFAULT_ADAPTERS)

That said, I could imagine some use cases for it. You could override a builtin (ie shell and campfire) adapter with your own in a different directory. You could even just 'override' it but have it do something completely different, without having the overhead of a package, like a new, private adapter.

Right now, I'm kind of inclind to stop exposing adapterPath in general, but I am definitely open to hearing feedback and use cases ✨

[technicalpickles]

I ended up fixing this in #1366

I am still interested in hearing about use cases, because I suspect that HUBOT_DEFAULT_ADAPTERS will limit how the custom adapterPath can be used.

[rqc]

@technicalpickles thanks for looking into this. And your ideas about potential use cases is on the right path of our use cases. Let me try to explain:

The main use case here is that we are using Hubot as a package and not as the driver / startup script for my bot. This is because we need to change / override some of that startup behavior for our company bots. Furthermore, we are adding our own default adapters for our bots. The issue arose when we were trying to plugin these custom adapters to our instance of Robot. The fact that it calls loadAdapter() in its constructor, and in turn loadAdapter is constrained by the default adapters and what can be specified in adapterPath and adapter caused some issues in how we created our new bot startup script and also how we tested our bots. We ended up overriding Hubot.Robot constructor and loadAdapter() so that we could instantiate a Robot instance without an Adapter instance and then load an adapter instance with our overridden loadAdapter() method.

In general, the design pattern of having robot include (and load) adapter and havingadapter also include robot has created issues for us in customizing our bots. I think it does not create a clear separation of concerns. If you have any suggestions on this I would be happy to hear them.

Thanks 👍

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.