ToothlessGear / node-gcm

A NodeJS wrapper library port to send data to Android devices via Google Cloud Messaging
https://github.com/ToothlessGear/node-gcm
Other
1.3k stars 206 forks source link

Debug module instead console.log #50

Closed ilongin closed 10 years ago

ilongin commented 10 years ago

I have replaced console.logs with small debug module which can be turned on or off by setting environment variable.

2fours commented 10 years ago

Would be better imo if you could pass in your own logger instance ala socket.io for example.

ilongin commented 10 years ago

This was just to avoid not needed console logs in production code. Personally I don't see why should someone use logs from dependency module other than debuging while developing and this is just fine for that purpose, but if you think it would be nice to have socket.io appender go for it ... I think it is overkill for this kind of module.

2fours commented 10 years ago

In a production system typically you want all the logging to go to the same place which this doesn't allow. If I'm using bunyan to log to a file for example then log statements from node-gcm are not going to appear in that file. I know it's a small module but I would still want to see node-gcm logging (especially error) statements inline with all the other logging statements in my server. I just think if one is going to make a change like this why not go all the way. Just my $0.02.

2fours commented 10 years ago

Perhaps it would just be better to have a flag to enable/disable console.log output and leave logging to the callback.

ilongin commented 10 years ago

Hm...I usually don't look for logs of dependencies in production but if you need to do that then it make sense.