Pass alias in Robot constructor. Fixes issue #1002.

[sdimkov]

This PR enables passing alias to robot's constructor. It fixes #1002. It also does not disturb in any way any existing 3rd party code as it doesn't change the behavior of robot's constructor without the extra param being passed.

But why?
Because both Robot name and alias should be provided during creation of the robot. In the current design they can not be changed later and are hard-coded into every listener's regex. Therefore modifying alias or name after the robot is created is wrong and could cause problems.

[michaelansel]

Based on what I see, I like it. I'm not too familiar with this part of the code though. I'll try to find time to read through and gain context...

[technicalpickles]

This makes sense to me.

One thought was about what else might be touching alias. It's usually checked for truthiness. Instead of a default false, it'd be getting undefined if you don't pass call it --alias but that's still falsy. That doesn't matter anyways since it's always getting assigned to robot.alias anyways, which would be undefined.

The other thought was that adding more parameters to the constructor, might be time to pass in an object rather than an array of parameters, but that can be a later refactoring.

[sdimkov]

it'd be getting undefined if you don't pass call it --alias

Nope :) That behavior is unchanged. You'd be getting false just as before.

That's because the new signature of the constructor is:
constructor: (adapterPath, adapter, httpd, name = 'Hubot',alias = false) ->
Which is also a bit safer than the current direct assignment of the field. Due to the nature of coffeescript's default value even explicitly passing a null or undefined results in having value false

On passing object to the constructor: That's an excellent idea for refactoring but I have one concern:
This is a breaking change as it alters the way Robot objects are created. Are there any third parties that construct robots programmatically? I believe there are at least some PRs on the subject.

[technicalpickles]

@sdimkov I was being a bit verbose to try to demostrate the type of things I look for in changes, but yeah, my conclusion was that it didn't change anything 😁

Are there any third parties that construct robots programmatically? I believe there are at least some PRs on the subject.

They will be pretty few and far between. hubot-mock-adapter is the only one that comes to mind. Probably could do some trickery with checking the typeof the arguments, if the first one is an object, treat it as having everything, otherwise, treat it the way it is now and print a deprecation message. Could also poke at the arrity of the function call too.

[sdimkov]

Okay, maybe we can merge this now ?

I don't mind writing the object-based constructor but that should be a separate PR.

[sdimkov]

Guys, it's sad to see this still being open.

I don't understand why fixing bugs is such a low priority here. I personally believe that stability and polish are at this point far more important than adding a new feature X or Y.

Is there anything more I can do to get this merged?

[michaelansel]

Yeah, I know... We're all just a little short on free time 😓

I think this looks good as-is, with the one possibility of adding a test case for the default alias state. What do you think @technicalpickles?