[MERGE 2/3] Towards XML log output: Refactor LogFormatter

John Arbash Meinel john at arbash-meinel.com
Sat Nov 4 18:39:49 GMT 2006


James Westby wrote:
> Hi,
> 
> The xml log format requires a little more context than the others as it
> needs to know when it has started and finished. The attached bundle
> refactors the LogFormatter class to make show_log and instance of it,
> and to store the interesting things in variables. This is more than is
> needed, but allows more flexibility. 
> 
> It deprecates show_log in it's original location and updates bzrlib to
> the new version.
> 
> James
> 
>

In the long run, I would like to see _show_log get split up a little bit
more. Since we now have a class to hold state, we don't need to keep
everything in the same function. Having things split up helps
maintainability and extensibility.

But I don't see any reason why this couldn't be merged as is. The
duplicated tests are a little bit weird, but I realize that is because
of deprecating the old function, so you left the test there, and just
copied it for the new test.

Because 'show_log' is just a thunk into LogFormatter.show_log, *I* would
just create a smoke test for it. Because there isn't any actual
compatibility code that would need to be tested. But I'm okay with what
you've done. +1 from me.

John
=:->


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 254 bytes
Desc: OpenPGP digital signature
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20061104/856fdefa/attachment.pgp 


More information about the bazaar mailing list