Bazaar

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

mutter-pid
Merge into bzr.dev

Revision history for this message

Eric Siegerman (eric97) wrote on 2011-01-08:

> So if you could [make the output narrower] that would be good. Ideally it would
> have more in common with how other thing are logged, not less.

Note that the non-mutter() lines are no wider than they were before; I haven't touched those, but only made the mutter() lines more closely resemble them.

The only remaining difference is that mutter() still prints the elapsed time (that's the number that was printed before my changes; it now follows "MUTTER:"). Removing the elapsed time would save a few characters (and one could get away with it now that the absolute date/time is included), but I assume that the it's there because it's useful, and so would be missed if it were to go away.

To reiterate, the status quo ante is format #1:
    1.234 Plugin name launchpad already loaded
    [21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

and this patch, as it stands, produces format #2:
    [21703] 2011-01-07 19:07:18.974 MUTTER: 1.234 Plugin name launchpad already loaded
    [21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

If I did remove the elapsed time, the output would look like format #3:
    [21703] 2011-01-07 19:07:18.974 MUTTER: Plugin name launchpad already loaded
    [21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

which is totally unified, but doesn't save all that many characters. Is it enough of an improvement to be worth sacrificing the elapsed-time field?

On the other hand, I could back part-way out, and add *only* the PID to mutter() lines. That would produce format #4:
    [21703] 1.234 Plugin name launchpad already loaded
    [21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

which is shorter, but violates your "more in common" request.

One could change the non-mutter() lines too, e.g. to include *only* elapsed times (format #5):
    [21703] 1.234 MUTTER: Plugin name launchpad already loaded
    [21703] 1.236 WARNING: bzr: warning: blah

But the non-mutter() lines are written by the Python logging subsystem, and hacking that might be beyond my current level of Python- and bzr-fu -- especially under Python 2.4, because the logging module's "extra" feature for adding custom fields only appeared in 2.5. I can look into doing that, but it'd take me a while. It might be better to land this branch, or a minor variant of it, and do the more extensive format changes as a follow-on branch if people feel they're warranted.

> So if you could [make the output narrower] that would be good.  Ideally it would
> have more in common with how other thing are logged, not less.

Note that the non-mutter() lines are no wider than they were before; I haven't touched those, but only made the mutter() lines more closely resemble them.

The only remaining difference is that mutter() still prints the elapsed time (that's the number that was printed before my changes; it now follows "MUTTER:").  Removing the elapsed time would save a few characters (and one could get away with it now that the absolute date/time is included), but I assume that the it's there because it's useful, and so would be missed if it were to go away.

To reiterate, the status quo ante is format #1:
&nbsp;&nbsp;&nbsp;&nbsp;1.234  Plugin name launchpad already loaded
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

and this patch, as it stands, produces format #2:
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 MUTTER: 1.234  Plugin name launchpad already loaded
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

If I did remove the elapsed time, the output would look like format #3:
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 MUTTER: Plugin name launchpad already loaded
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

which is totally unified, but doesn't save all that many characters.  Is it enough of an improvement to be worth sacrificing the elapsed-time field?

On the other hand, I could back part-way out, and add *only* the PID to mutter() lines.  That would produce format #4:
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 1.234  Plugin name launchpad already loaded
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 2011-01-07 19:07:18.974 WARNING: bzr: warning: blah

which is shorter, but violates your "more in common" request.

One could change the non-mutter() lines too, e.g. to include *only* elapsed times (format #5):
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 1.234 MUTTER: Plugin name launchpad already loaded
&nbsp;&nbsp;&nbsp;&nbsp;[21703] 1.236 WARNING: bzr: warning: blah

But the non-mutter() lines are written by the Python logging subsystem, and hacking that might be beyond my current level of Python- and bzr-fu -- especially under Python 2.4, because the logging module's "extra" feature for adding custom fields only appeared in 2.5.  I can look into doing that, but it'd take me a while.  It might be better to land this branch, or a minor variant of it, and do the more extensive format changes as a follow-on branch if people feel they're warranted.

« Back to merge proposal