[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