[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
Comment:
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 =
LogRevision(rev,revno,merge_depth,delta,rev_tag_dict.get(rev_id))

There should always be a space after the commas.

In normal text you have:

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

Similarly:
+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
__init__(self,rev=None,revno=None,merge_depth=0,delta=None,tags=None):

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: 
http://bundlebuggy.aaronbentley.com/request/%3C4641DADE.7020806%40gmail.com%3E



More information about the bazaar mailing list