[MERGE] (0.17) reworked LogFormatter API

John Arbash Meinel john at arbash-meinel.com
Thu May 24 12:46:38 BST 2007

John Arbash Meinel has voted +1 (conditional).
Status is now: Conditionally approved
You have some minor PEP-8 violations. Things like:
-    if show_merge is not None and show_merge_revno is None:
+    legacy_lf = not getattr(lf,'log_revision',None)

should be:

-    if show_merge is not None and show_merge_revno is None:
+    legacy_lf = not getattr(lf, 'log_revision', None)

I would also probably do this as:

legacy_lf = getattr(...) is not None

Same thing here:
+            lr =

There should always be a space after the commas.

In normal text you have:

x = function(a, b, c=d, e=f)
                ^--^- spaces
                    ^^^- no spaces for named parameters

+class LogRevision(object):
+    """A revision to be logged (by LogFormatter.log_revision).
+    A simple wrapper for the attributes of a revision to be logged.
+    The attributes may or may not be populated, as determined by the
+    logging options and the log formatter capabilities.
+    """
+    def

should be:

      def __init__(self, rev=None, revno=None, merge_depth=0,
                   delta=None, tags=None):

same thing later on in your import:
-from bzrlib.missing import find_unmerged, iter_log_data
+from bzrlib.missing import(
+    find_unmerged,
+    iter_log_revisions,
+    )

should be 'from bzrlib.missing import ('

I'll go ahead and do the cleanups, and merge this, since it took so long 
to get it reviewed.

For details, see: 

More information about the bazaar mailing list