less / more

less on rails — the official LESS plugin for Ruby on Rails
http://lesscss.org
MIT License
226 stars 41 forks source link

Problems caused by setting logger to nil #13

Closed jxson closed 15 years ago

jxson commented 15 years ago

Hey guys,

Great work!

I noticed that when I remove a css file and then try to load it in the browser (without restarting the browser) I was getting and error from LessCacheController where it was setting logger to nil.

The problem is that I have some before filters that use the logger, I was able to resolve it by removing the method. My fork is here, and the tests pass: http://github.com/jxson/more

augustl commented 15 years ago

Your case seems to be an edge case, and having the logger on for the stylesheets controller will really clutter up the logs in development mode, so I'll leave this as it is.

jxson commented 15 years ago

That's a good point however, I have the logger being used in a before filter that sets the locale which uses debug.

In my app things log to debug in development and test mode only, I believe this is the Rails default. Using logger.debug makes sure that log files do not get cluttered in production while providing valuable debug info for the app when in the development cycle.

Setting the logger to nil will raise errs in cases where the logger is being used in a normal and expected fashion anywhere in the request cycle for the css controller (before, after, and/ or around filters in the application controller for instance).

Perhaps it would be better to actually silence the logger using an around filter or something instead of setting it to nil?

I'll do some research and see if I can come up with a decent solution that will both silence the logger as well as not break it.

augustl commented 15 years ago

Action Controller itself checks if the logger object is nil before it does anything with it, so the only case where it'll be a problem is with plugins and such, as far as I know.

If silencing the logger is possible, that would be awesome. I'm not that into the Logger class, really. If you can figure out a way to do this, it'd rock.

One solution would be to create a logger that logged to /dev/null, I guess :)