[MERGE][Bug #162469] log displayers for custom revision

Guillermo Gonzalez guillo.gonzo at gmail.com
Sat Mar 15 12:32:03 GMT 2008


Robert,
    Thanks for the comments.
    I'm going to add both test cases, and fix the comments and docstrings.

Cheers,


On Fri, Mar 14, 2008 at 1:56 AM, Robert Collins
<robertc at robertcollins.net> wrote:
> Robert Collins has voted resubmit.
>  Status is now: Resubmit
>  Comment:
>  A few issues.
>
>  some of your docstrings do not have a proper sentence as the first line
>  (see PEP-8/HACKING)
>
>  This hunk:
>  @@ -664,6 +681,9 @@
>               to_file.write('\n')
>               for parent_id in revision.rev.parent_ids:
>                   to_file.write(indent + 'parent: %s\n' % (parent_id,))
>  +
>  +        # show custom properties
>  +        self.show_properties(revision.rev.properties)
>
>           author = revision.rev.properties.get('author', None)
>
>  The comment is not needed, nor is the extra vertical whitespace. (Too
>  many comments can detract from readability by making the code area not
>  fit on screen compactly.).
>
>  Lastly you should test error cases - what happens if there is a bad
>  plugin function, will bzr blow up/degrade gracefully/undefined? Does log
>  work correctly with no plugins present (you have to explicitly test this
>  because selftest loads plugins).
>
>
>

-- 
Guillermo



More information about the bazaar mailing list