Developers of Sympa

[Guillaume Rousse <address@concealed>]

Le 05/08/2013 16:41, IKEDA Soji a écrit :
> Hi,
>
> There are some points I realized about Log functions: one notice and
> three questions.
>
> - About parameters for do_log().
>
>    Notice: do_log() and so on use sprintf() internally.  So in
>    general interpolation may cause unexpected result.
>
>      Sympa::Log::do_log('err', "Could not open $file");
>
>    should be:
>
>      Sympa::Log::do_log('err', 'Could not open %s', $file);
Or the reverse, but I agree usage should be consistent.

> - Unimplemented 'warn' level.
>
>    Developers' Howto
>    http://www.sympa.org/dev/contributors/developers_howto
>    tells about 'warn' log level.  It is not implemented.
>
>    I suppose 'warn' level might be useful for users, however,
>    what is difference from 'err' level (especially from users' view)?
Rather than yet-another-not-so-defined-log level, I'd rather reduce them 
(for instance, merging debug, debug2 and debug3), and try to make a 
better distinction between user-targeted runtime messages, and 
developper-targeted debugging messages. User don't care about source 
file, parameter values, or calling function, for instance, even in case 
of error, they are just interested in "what did go wrong" information.

The current unique all-purpose do_log() function should probably 
splitted in log_message() and log_trace_message() functions, the latter 
being just a wrapper over the first one.

> - fatal_err() vs. croak().
>
>    fatal_err() seems to be replaced by croak() in most cases.
>    Former requires Sympa module, while latter needs just a standard
>    Carp module.
>
>    What are pros to use fatal_err()?
It just predates usage of exceptions, and should better be dropped in 
favor a exception handler in top-level code.

> - Three types of logging.
>
>    Sympa::Log::Syslog::do_log(), Sympa::Log::Database::do_log() and
>    wwsympa::web_db_log() are co-exist and they are not always used
>    all at once.
>
>    Are separate logging functions needed for each logging store?
From what I understood so_far, Sympa::Log::Database isn't an 
alternative for general purpose logging, but serves a distinct purpose. 
I planned to rename the module as Sympa::PerfLogger, while splitting 
Sympa::Log to Sympa::Log::Syslog and Sympa::Log::Stderr.

wwsympa::web_db_log() seems to be just a wrapper over Sympa::Log, 
causing this last one to use a ugly 'if-im-called-from-web_db_log' 
conditional block when trying to identify its caller.

--
Guillaume Rousse
INRIA, Direction des systèmes d'information
Domaine de Voluceau
Rocquencourt - BP 105
78153 Le Chesnay
Tel: 01 39 63 58 31

Attachment: smime.p7s
Description: Signature cryptographique S/MIME