Code review comment for lp:~eric97/bzr/mutter-pid

Revision history for this message
Martin Pool (mbp) wrote :

On 10 January 2011 16:51, Eric Siegerman <email address hidden> wrote:
> On Sun, 2011-01-09 at 02:16 +0000, Martin Pool wrote:
>> On 8 January 2011 09:26, John A Meinel <email address hidden> wrote:
>> > I don't find the absolute date-time to be very useful, when with
>> > pid+elapsed time+log at startup time you can get it if you need to.
>
> I've got Python logging to emit the elapsed time.  Not nearly as
> hard as I'd thought; I just hadn't found the right knob yet :-)
>
> That means that format 5 above is feasible.  To repeat it:
>    [21703] 1.234 MUTTER: Plugin name launchpad already loaded
>    [21703] 1.236 WARNING: bzr: warning: blah
>
> Or leaving out the "MUTTER:" gives format 6 (but see below):
>    [21703] 1.234 Plugin name launchpad already loaded
>    [21703] 1.236 WARNING: bzr: warning: blah
>
>
>> Possibly we should only log the full time when it's for say a server
>> log, and then it should probably go to syslog.  For the common cases
>> of just .bzr.log it should not run long enough to matter.
>
> I think what you're saying is that:
>  - "bzr server" and the like should write to syslog instead of to
>    .bzr.log (in which case, IIUC, the format isn't bzr's
>    decision, but rather the syslog server's)
>
>  - other subcommands should write (only) elapsed times to .bzr.log

Right. Actually sending it to syslog is probably a job for a
different patch. On Windows, syslog is probably not the right choice
- we'd want a file or the MS logging infrastructure.

> Does bzr currently log to anywhere other than .bzr.log?  If so,
> obviously I need to avoid breaking that.

I don't think so, except there is some special handling in the test suite.

>> The word "warning" has some meaning to the user to say
>> that it's not been fatal etc, but "mutter" is just a random internal
>> bit.
>
> True.  What the "MUTTER:" does do, though, is eliminate a special
> case from the log-file syntax, and in general I dislike special
> cases and try to avoid them.  In particular, I find log files
> easier to read if the columns line up; and if I want to use a
> complicated grep on one, or even slurp it into a spreadsheet,
> variant line syntaxes need to be worked around.  (Admittedly,
> this last point is more important for long-lived servers.)

I see your point. How about making it 'debug' which is more
meaningful to users, and matches the standard severity scale? Maybe
even lowercase for all of them.

--
Martin

« Back to merge proposal