[MERGE] tags in log output

[John Arbash Meinel]

Kent Gibson wrote:
> 
> 
> Alexander Belchenko wrote:
>> I'd like to use kwargs, because creating new object for each
>> revision could leads to slowdown without any real benefit. We can
>> document all new parameters (expected in kwargs) for log formatter
>> in the API docstrings.
> 
> After all the other crud log.py has waded through to work out what
> revisions to log, I seriously doubt an extra class creation per
> revision would make any noticeable difference to the speed.
> 
> I agree the params could also be documented in either the method
> itself, or in the API in general.  I haven't actually paid much
> attention to how bzr code is documented so I don't have much to base
> an opinion on, though I probably have a stronger bias toward using
> classes than the average bzr developer.
> 
> I'd be interested to know if one approach has an edge wrt marshalling
> the params by show_log and demarshalling them by the LF, as measured
> by code simplicity and efficiency.
> I suspect the two are equivalent, and lean towards using a class cos I
> feel provides a cleaner encapsulation than a kwargs dict, and allows
> us to add helper methods to LogRevision that may otherwise end up
> being replicated in the log formatters themselves.
> 
> Cheers,
> Kent.

At this point in the code, I would rather be working in cleaner apis
(classes and objects). If we find performance issues, than we can
address them.

My general feeling is that it depends on how much data the algorithm
ends up needing to perform. If it is something that has to operate on
the *whole* data (whole tree, all revisions, etc) then it should be
optimized and avoid python objects. But if it can work on just a subset
(only changed files, etc) then we should be dealing in clean API's.

So for example, WT._iter_changes(). Underneath, it should be using
tuples because it has to compare two complete trees. So it needs to
process all files on both sides.

But above _iter_changes() we should have culled down to a relatively
small subset of the tree.

I would actually say that _iter_changes should probably be returning
smarter objects rather than tuples. I know *I* find that programming
against _iter_changes is pretty difficult. I keep jumping back to say
"now what is the third parameter again?".

As for 'log', it technically handles all revisions, but really it only
needs to handle a few revisions before the screen is full, and a user is
(temporarily) satisfied. As long as we get those first revisions quickly
(which Aaron did very good work on) we should be fine.

John
=:->