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

[John Arbash Meinel]

James Westby wrote:
> On (04/11/06 12:39), John Arbash Meinel wrote:

...

> Hi John,
> 
> Thanks for the comments,
> 
> I have attached an updated bundle that addresses your concerns. It first
> simplifies the tests, I agree that the previous solution was ugly. It
> then attempts to split _show_log up, I thought about doing this before,
> but didn't feel confident I would do it right. I'm not sure I broke it
> in the right places. I just grabbed the logical chunks and pulled them
> out. Also _show_log is still called on its own, rather than calling a
> group of functions to get the job done, I don't know if that is what you
> wanted.
> 
> Also I was tempted to pull some of the other functions in to be
> members, but I didn't do that. Let me know if it would be wanted. 
> 
> As always comments are welcome.
> 
> Thanks,
> 
> James
> 

Aaron's comment on Bundle Buggy is:

Aaron Bentley: -0
I'm not super enthusiastic about this, because you seem to be assuming
that every log formatter is a derived from LogFormatter.   Duck typing
means that's not required.   So this isn't really maintaining interface
compatibility.


I can understand his point, though I think we need to discuss it.

At some point we have to be allowed to do something like this. But maybe
we could change the code to do:

def show_log(...):
  log_func = getattr(lf, 'show_log', None)
  if log_func is not None:
    return lf.show_log(...)

  symbol_versioning.warn('Log formatters should provide a show_log'
			 'function as of version ...')
  ... do it the old way ...


And then for the moment, we won't change the callers, so builtins.py
still calls bzrlib.log.show_log(lf=lf).

And then after a release or 2, we can switch to just calling
lf.show_log() directly.

Would that work better for you Aaron?

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/20061110/41f27e12/attachment.pgp