Feature/logger refactor

[qejk]

Refactors Space.Logger, its initialization, and removes unnecessary use of Space.log global var. Enables to pass multiple transports to underlying library for logging (winston) through application config like:

My.App = Space.Application.define('My.App', {

  configuration: {
    appId: 'My.App',
    log: {
      enabled: true,
      winston: {
        console: {
          enabled: true,
          minLevel: 'info'
        },
        transports: [
          new winston.transports.Console({
            name: 'console',
            colorize: true,
            prettyPrint: true
          })
        ]
      }
    }
  },
})``` 

[rhyslbw]

So we can do away with the Space.configuration as that was only needed since we were previously initialising the logger instance outside the module chain. This will be really nice to see gone

[rhyslbw]

Nice start @qejk , really excited to see this design fleshed out!

cc @DominikGuzei @darko-mijic
Adam and I have been discussing this improvement to clean up a few issues with the first implementation of the logger and to allow for flexibility with transports. As part of this change we also scoped the ability to disable the built-in console logger.

Please review

[DominikGuzei]

Great work here, the only thing i'm concerned with is the default logger complexity this brings into space:base as we discussed, it would be better to move into more modularity instead. What do you think?

Another problem is the new code in Coffeescript 😜 we want to move to ES6

[rhyslbw]

We did discuss the idea of breaking out the console (default) logger, but deferred the idea, as the first step was just to make the code modular. It's a little more complex due to the fact that space:base calls the log method and it's within the module init that we want a logger instance created, but now that @qejk has introduced the concept of an adaptor the base logger class is essentially a noop logger. So we could move the server and client logger implementations into a separate package/s which removes the winson dependency in space:base, and makes it easy to add logging only if desired (or even write your own adaptor).

I vote for the approach with the client and server adaptors in separate packages, because in production a client logger makes little sense, so can see it purely as a development tool.

Also @qejk will be converting to ES6 when the design is fleshed out.

[DominikGuzei]

Yes, moving this into a general purpose configurable logger package that has no idea about Space will be the best option. This is the way forward!

Remember, packages like this are the goal:
https://github.com/meteor-space/template-controller

very small, focussed and highly documented (because it's easy!)

[qejk]

@DominikGuzei Changes will be moved to ES6 when I'm fully happy with them. Don't have much time currently and I'm faster with CS when prototyping.

Normally this would be like Rhys said - noop logger that can handle multiple logging libraries with granular control over them however developer pleases(and by that how he write his own adapter wink wink).
If there is transport that winston does not support - someone can use his own library. The big question is: if we actually require such thing in first place, since specs for that change initially were quite simple.

With JS there is limited way how to handle things because of lack of interfaces, so some sacrifices must be made when using typical OOP patterns, so would like to hear if this is the right direction to go.

Currently I would like to refactor only this ugly pice of code:

base/source/module.coffee

Line 197 in 64f90e0

_loggingConfig: () ->

and ensure that everything else related to initialization of Space.Logger is ok around with module vs app initialization, since I know only how things works partially.

Moving this to another module will require rethinking everything from ground up, since

base/source/module.coffee

Line 30 in 64f90e0

@log.debug("#{@constructor.publishedAs}: initialize")

debugging starts before module initialization.

[DominikGuzei]

Yep, that's another possibility – make it configurable in a way that space:base does not depend on winston and anyone can use whatever they like 😉

[rhyslbw]

@qejk Do you have .eslintrc checking the format? This should be a new line