Skip to content
This repository has been archived by the owner on Mar 18, 2022. It is now read-only.

Added support for custom logger functions #6

Merged
merged 3 commits into from
Jul 5, 2012
Merged

Added support for custom logger functions #6

merged 3 commits into from
Jul 5, 2012

Conversation

jameswyse
Copy link
Contributor

Adds the ability to pass a custom logging function as an option.
The default is still console.log()

I use the excellent winston library to manage logs, so in this case I would pass my winston log function to express-cdn:

var options = {
  // other options
  logger: winston.info
}

var CDN = require('express-cdn')(express, options);
express.dynamicHelpers({ CDN: CDN });

Adds the ability to pass a custom logging function as an option.
The default is still console.log()
@niftylettuce
Copy link
Collaborator

Great work with everything, ill look over today 👍
On Jul 5, 2012 4:06 AM, "James Wyse" <
reply@reply.github.com>
wrote:

Adds the ability to pass a custom logging function as an option.
The default is still console.log()

I use the excellent winston
library to manage logs, so in this case I would pass my winston log
function to express-cdn:

var options = {
  // other options
  logger: winston.info
}

var CDN = require('express-cdn')(express, options);
express.dynamicHelpers({ CDN: CDN });

You can merge this Pull Request by running:

git pull https://github.com/jameswyse/express-cdn custom-logger

Or you can view, comment on it, or merge it online at:

#6

-- Commit Summary --

  • Added support for custom logger functions

-- File Changes --

M lib/main.js (9)

-- Patch Links --

https://github.com/niftylettuce/express-cdn/pull/6.patch
https://github.com/niftylettuce/express-cdn/pull/6.diff


Reply to this email directly or view it on GitHub:
#6

@emostar
Copy link

emostar commented Jul 5, 2012

Using winston is definitely a good idea, I wouldn't be against making it be a requirement in fact...

@@ -376,6 +380,11 @@ var CDN = function(app, options) {
throwError('missing option "' + options[index] + '"');
});

if(options.logger) {
Copy link

Choose a reason for hiding this comment

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

Not to be nit picky... but I will anyway :P

if (options.logger) { would match the format used in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hah, you're right. The shame!

Though really we should work on making the whole code more modular, i've another bug to solve involving timestamps in filenames and the code is a little confusing at present.

@niftylettuce
Copy link
Collaborator

I'll manually fix the format per @mathrawka suggestion \o

Excited to use loggly/winston together with this!

niftylettuce added a commit that referenced this pull request Jul 5, 2012
Added support for custom logger functions
@niftylettuce niftylettuce merged commit 21601d4 into ladjs:master Jul 5, 2012
lxe referenced this pull request in lxe/express-cdn Aug 1, 2013
Moved the hashed filename creation after the CSS is transformed/replaced
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants