Question: node-mumble module as Class

[mikemimik]

I created a fork which turns the node-mumble module into a class which can be instantiated and then be called to connect at a later point in code. Would this be something you're interested in? If so I'll create a PR, if not I'll just continue to maintain the fork.

[Rantanen]

That sounds useful. The MumbleConnection is already exposed through require( 'mumble' ).MumbleConnection, but the MumbleClient, which was added later isn't.

Ideally we should probably include the connect() functionality in the MumbleClient and rewrite the index.js to expose both the connect() as it does now, but also the MumbleClient in similar way to MumbleConnection.

Is this anywhere close to what you've done?

[Prior99]

@Rantanen this sounds good. I would appreciate it if we would not take breaking changes into the repository and keep the old .connect() method which then just instanciates the class.

[tjhorner]

I agree with @Prior99, as long as the front-facing API isn't changed I totally support this change.

[mikemimik]

My changes simple add the ability to instantiate the module and decouple the connect() function with its callback into the instantiated object.

'use strict';
const mumble = require('mumble');
const ConnMngr = mumble.ConnectionManager;
let connMngr = new ConnMngr(url, options);
connMngr.connect(function(err, client) {
  /* still functions as normal */
});

I'll create a PR and reference it here.

[mikemimik]

I'm open to discussion on how to implement this but I needed this functionality for a project I'm working on. It was necessary to setup the module before connecting it to the service.

[Rantanen]

@Prior99, @tjhorner I believe both of you have existing implementation against the module? If you have time, I'd appreciate if you were able to test whether updating this module to version 0.3.9 (which I just published to npm) breaks anything?

The old connect method should still work and the old tests did pass, but I would hate to find out that I introduced some breaking change in a couple of months. :)

[tjhorner]

@Rantanen Sure thing, I'll try it on one of my Mumble bots and see if it works.