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

bot: use a logger properly, pass it to modules #143

Merged
merged 6 commits into from
Jun 28, 2017
Merged

bot: use a logger properly, pass it to modules #143

merged 6 commits into from
Jun 28, 2017

Conversation

euank
Copy link
Member

@euank euank commented Jun 26, 2017

@LinuxMercedes This is the first pass at proper logging.

The big TODOs remaining are:

  • Actually use it from all modules that currently console.log
  • Allow an administrator to configure per-module loglevels

I'm fine punting those to future PRs since I think this is still somewhat better already

@@ -85,6 +86,10 @@ me.loadModules = function(cb) {
me.initModules = function(cb) {
// Todo, don't ever init if they're already initted
_.each(_.values(modules), function(mod) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you modify this loop to capture the key (module name) of each module? That would eliminate the moduleName function below.

@LinuxMercedes LinuxMercedes mentioned this pull request Jun 27, 2017
23 tasks
@LinuxMercedes
Copy link
Member

I know this project aggressively follows "the code is the documentation" (right?) but could you say a few words in #144 about how to use the logger?

It was redundant. This does introduce a new `module.exports._name`
variable set for each module to keep track of that, but this doesn't
seem too egregious.
This shows all supported options.
@euank euank merged commit b9f90e4 into master Jun 28, 2017
@euank euank deleted the logging branch June 28, 2017 04:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants