-
Notifications
You must be signed in to change notification settings - Fork 2
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
Fazer melhor uso do logger #28
Comments
Hi Andre, I've been allocated this module as part of the CPAN PR Challenge and this issue looks like something I might be able to help with. My understanding (from Google Translate!) is that you would prefer to have a replacement of or addition to EmptyLogger where the methods are non-empty and actually throw exceptions. This should then be used in place of the core warn() and die() throughout the rest of the codebase to allow uniformity of error logging. If that's correct, could I suggest maybe SimpleLogger or BaseLogger or even just Logger as the package name? I would prefer to make sure that we are in agreement about precisely what you want to achieve with this change before starting any active development on it. Thanks for any more detail you can provide. Pete |
Hello, Pete! Sorry for the delay! I was on vacation, and I forgot completely about this! Also, sorry for writing this down in Portuguese. At the time, I was developing this with a few other Brazilians, so I figured that "sketching" my ideas here would make more sense in Portuguese 😄 Alright, so about the idea itself. So the But I don't want to depend on any of those modules. I want to keep it simple, and if no custom logger was defined, I want it to work just as well. That's why I created EmptyLogger. I wanted it to be dead simple, as you can see in the source code, because I don't want to reinvent the wheel. Anyway, before the idea of the logging, I had written a few warn's, die's around. I figure that these should be wrapped around that default logger. But, if that happens, it shouldn't be EmptyLogger anymore :P But you're right, SimpleLogger, or just Logger sounds much more reasonable. Does that make sense? Do you think it's reasonable, or would you like to try something else? Note that I have other drivers that were developed internally in the company (not published on CPAN) for legal reasons. That's why sometimes a few features in the core module seem unused in the available drivers (e.g. PayPal, PagSeguro, etc). Again, I'm really sorry about the delay. Kudos for joining the PR challenge, and good luck with that! If you need any further help with this, please call. I'll try to answer more promptly :) |
Thanks for all this extra detail. That makes good sense to me and I think that perhaps SimpleLogger is therefore the best name as it suggests limited functionality and the user is free to replace it in the constructor with Log::Log4perl or similar if more features are required. I will give that a go and get back to you if further questions arise. Don't worry about the delay in replying - everyone needs a holiday and I've been busy with other things anyway. And please don't apologise for writing the original issue in your native language! I'm afraid that my Portuguese is non-existent so my comments in the code and replies here will all be in English. |
Great. Looking forward to see your PR :) Thank you, @openstrike! |
A quick question: I had assumed that $log->fatal('foo') should terminate (ie. die()) and have written it as such. However, reviewing the Log::Log4perl docs, it appears that their fatal() method does not die, and that for this they use logdie() instead. Do you want to replicate this in SimpleLogger and have both fatal() and logdie() as separate methods or do you want the one fatal() method which would die automatically? |
Parar de usar "warn" e "die" por toda parte, e chamar sempre $cpi->log->warn(), $cpi->log->error() no lugar disso. Trocar o EmptyLogger, por um AlmostEmptyLogger (com um nome melhor, evidentemente).
The text was updated successfully, but these errors were encountered: