Skip to content
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

Open
andrewalker opened this issue Feb 25, 2014 · 5 comments
Open

Fazer melhor uso do logger #28

andrewalker opened this issue Feb 25, 2014 · 5 comments
Milestone

Comments

@andrewalker
Copy link
Owner

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).

@andrewalker andrewalker added this to the v1.0 milestone Feb 25, 2014
@openstrike
Copy link

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

@andrewalker
Copy link
Owner Author

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 log attribute works like this. Suppose Business::CPI is used in a Catalyst application, one could instantiate it like this: Business::CPI->new( ..., log => $c->log);, where $c is Catalyst's context. That way, all logging goes to the same place, and if debug flag is turned on in Catalyst, Business::CPI would log debug messages too, etc. Also, if the application uses Log::Log4perl, or Log::Dispatch, it could be shared too, etc, providing several advanced options for customizing the log.

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 :)

@openstrike
Copy link

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.

@andrewalker
Copy link
Owner Author

Great. Looking forward to see your PR :) Thank you, @openstrike!

@openstrike
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants