Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change the way logging is done #55

Open
satazor opened this issue Feb 17, 2013 · 5 comments
Open

Change the way logging is done #55

satazor opened this issue Feb 17, 2013 · 5 comments
Labels

Comments

@satazor
Copy link
Member

satazor commented Feb 17, 2013

At the moment, logging is done via the context. To increase compatibility with the upcoming node-task, logging could be done via an emitter like so:

this.emitter.emit('log', 'hello');
this.emitter.emit('log.info', 'hello info');
this.emitter.emit('log.warn', 'hello warn');
this.emitter.emit('log.success', 'hello success');
this.emitter.emit('log.error', 'hello error');
this.emitter.emit('log.debug', 'hello debug');

Question: Should we remove the ln versions or do it like this:

this.emitter.emit('logln', 'hello');
this.emitter.emit('logln.info', 'hello info');
this.emitter.emit('logln.warn', 'hello warn');
this.emitter.emit('logln.success', 'hello success');
this.emitter.emit('logln.error', 'hello error');
this.emitter.emit('logln.debug', 'hello debug');

Please note that, the context parameter could be removed completely. Functions would only receive opts and next.

--- Want to back this issue? **[Post a bounty on it!](https://www.bountysource.com/issues/107105-change-the-way-logging-is-done?utm_campaign=plugin&utm_content=tracker%2F39683&utm_medium=issues&utm_source=github)** We accept bounties via [Bountysource](https://www.bountysource.com/?utm_campaign=plugin&utm_content=tracker%2F39683&utm_medium=issues&utm_source=github).
@satazor
Copy link
Member Author

satazor commented Feb 17, 2013

//CC @tkellen

@tkellen
Copy link

tkellen commented Feb 17, 2013

+1 on using an emitter!

@cowboy has a lot more experience dealing with building nice loggers, so I'd probably defer to him on this. The logger will have a spec also, but we haven't even approached that yet.

@millermedeiros
Copy link

I would probably not namespace the events (use info, instead of log.info).

there are many loggers on npm, should be trivial to hook any of the existing loggers:

var task = require('myTask');
// just use the native `console.log` method
['log', 'error', 'info', 'warn', 'debug'].forEach(function(level){
  task.on(level, console.log.bind(console, level+':');
});

each runner would take care of implementing a logger, that way we could reuse the same tasks in multiple places (not only on build scripts), sometimes these events might be used by other means besides logs - e.g. each time error is dispatched it also stops the execution of other tasks and exists with statuscode 1... (that would also be a responsibility of the runner, not the task)

@satazor
Copy link
Member Author

satazor commented Feb 20, 2013

@millermedeiros thanks for the feedback.
There might be cases in which the user wants control over linefeeds. If we decide to not differentiate them at the event name, people would need to:

this.emit('log', 'Hi, Ive a new line\n');
this.emit('log', 'And Ive not');

This could cause wrong assumptions about the default behaviour since, in the users mind, the event names would have a direct correlation with the console.* counterparts. But the console.* methods always print new lines. What do you think about this issue?

@tkellen
Copy link

tkellen commented Feb 20, 2013

I like the implementation @satazor posted originally--I definitely want control over newlines when logging and I don't mind the namespaced events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants