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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 1/8/2011 3:03 AM, Eric Siegerman wrote:
> Sigh: let's try that again without the  's. No other changes...
>
>> 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.

^- This is my personal favorite. mutter lines are far more common than
warning/error lines.

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.

[PID] has some real use cases for sorting out which lines belonged to
which process. So I'm in favor of adding it.

John
=:->

>
> 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.
>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk0ogMgACgkQJdeBCYSNAAMZngCgnP1V6ofK4nk9YJ+cetGt1BqS
Q0UAnRlI9/SFC3ipdj0U9sfksakvXDDV
=cqga
-----END PGP SIGNATURE-----