[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