-
Notifications
You must be signed in to change notification settings - Fork 7
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
Fuel logger logic moved to a handler #3
Comments
What do you mean exactly? Logging is done by Monolog? |
The logic in the log config should IMO go into a Monolog Handler. So you can do |
Ah, you mean the test code in the config/log file? Yes, absolutely, it's been put there to have some logging, it definitely has to move elsewhere. |
Yes, I mean that. I suggested a handler for easy reuse. |
I agree, but hadn't had the time to do it. So if you do, be my guest. ;-) |
I usually put extensions like this in the original library's namespace, so this would be Monolog\Handler\FuelHandler. Is it OK for you? If not, where would you like to see it? |
Haven't thought about it to be honest. The potential issue I can see is that people using this may assume it's part of the Monolog package, based on the namespace. I probably would turn it around, and use \Fuel\Foundation\MonologHandler or something (or move it into a Log namespace if we intend to have multiple log handlers). I think we need to good discussion about bootstrapping the framework. Due to the static nature in v1, using config files returning array's of scalar values was fine. But v2 is a lot different, and perhaps we have to revise this, and go for more "active" configuration, like is now done for Environment and Log (and to a lesser extend for Routes, I think that needs changing too). I have to say I quite like the closure based config, it provides a lot more flexibility to use code to provide configuration values, without having to go through the complexities of class extensions... |
My concerns with the closure config is that you can only make it working with a php config file. Furthermore if you do get an a specific config item by accident, based on the closure's argument you get an error, which you can't handle (you cant control what people do with closures, so its better not relying on it). But I also think it is easy and some timee I can't avoid using it. In production env (where you possibly use opcode cache) it can be a pain using php configs (hence the stupid cache invalidation). |
hmm... yes, that is a very valid point. One more reason to have this discussion. ;-) |
I would like to see the whole logger file creation logic in a FuelHandler for easy reuse.
Opinions?
The text was updated successfully, but these errors were encountered: