Add debug config option for irc-fw debug log

[maxpoulin64]

Because ifs are too dangerous for pre5.

[astorije]

When merged, this will close #459.

First of, two things that need checking:

• Existing configuration files will work with this. This should be ensure by Cache loaded config and merge it with defaults #387, but I just want to make sure we are aware of it.
• Should the if condition be inside or outside that handler? Since the handler is only about logging debugs at the moment, it should be fine as is, but I'm curious if that could lead to any kind of race condition, should the config helper be or become any async.

Are there any other place where it's worth logging debugs at the moment?
I know @williamboman suggested we set process.env.NODE_ENV === 'development' as well, what are the consequences of this?

[maxpoulin64]

Actually, if it's not set it will be undefined so it won't pass the if anyway. Same as the default, so we're fine even without the new config merging feature.

For now config can't change at runtime so I think it is fine that way. Eventually we probably will want that to change, but we'll also probably want config change listeners so we'll probably change it anyway to dynamically on/off the debugging as well.

As for process.env.NODE_ENV === 'development', I guess we could use it as the default when debug is undefined. That way the config would override it, but by default it would make sense in most node app hosts. Sounds good?

[astorije]

As for process.env.NODE_ENV === 'development', I guess we could use it as the default when debug is undefined. That way the config would override it, but by default it would make sense in most node app hosts. Sounds good?

Hmm, not sure I understand. Does that mean that in all currently deployed instances of The Lounge, unless they update their config file, will be run in dev mode after upgrading? That seems extreme :-)

The real question is... what does process.env.NODE_ENV = 'development' even do in the first place?! I have found many interpretations, but nothing too authoritative.
Also, I do have concerns about mixing environments and log levels. One might want to raise log level in production, or suppress debug logs in development... so unless there is a very good reason to do so, I'd leave it out at least for now.

Regarding other places to log, when it comes to irc-framework, it looks like this debug event is the only thing the lib sends us so we're good.
For the others, I suggest going through other packages and see if they do something similar. I'm thinking more specifically of socket.io, express and request, but maybe others too.

[astorije]

Hey @maxpoulin64, could you rebase this on master please?

After sleeping on it, regarding process.env.NODE_ENV = 'development', I don't believe it belongs there for the reason detailed above: dev/prod environment and verbosity/debug level are 2 different things.

Regarding debug modes of other dependencies, we can either take a look now or add them later in a different PR if needs be. I don't have a strong opinion on that.

[astorije]

Rebased as there was simply a conflict on defaults/config.js due to #477.

👍 and merging.