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

Ian Clatworthy ian.clatworthy at canonical.com
Wed Jul 9 07:37:07 BST 2008


Guillermo Gonzalez wrote:

>> Any comments are more than welcome.

One coding bug and some minor doc things to clean-up. Another test requested
too.

bb:resubmit

> +    * New registry for custom log displayers and the method in 
> +      LongLogFormatter to display the custom properties returned by the 
> +      registered handlers. (Guillermo Gonzalez, #162469)

This NEWS item is fine but I'd prefer to see it under IMPROVEMENTS instead of
BUGFIXES.

> +    Plugins can register functions to show custom revision properties using
> +    the custom_properties_handler_registry. The registered function

You mean properties_handler_registry here, not custom_properties_handler_registry.

> +        If a registered handler raises a error it is propagated.

s/a error/an error,/

> +        for key, handler in properties_handler_registry.iteritems():
> +            for key, value in handler(revision).items():
> +                self.to_file.write(indent + key + ': ' + value + '\n')

I wonder if you ought to be displaying these in sorted order. Any thoughts?

> +class TestCaseWithoutPropsHandler(TestCaseWithTransport):
> +
> +    def setUp(self):
> +        super(TestCaseWithoutPropsHandler, self).setUp()
> +        # keep a reference to the "current" custom prop. handler registry
> +        self.properties_handler_registry = \
> +            log.properties_handler_registry
> +        # clean up the registry in log
> +        log.properties_handler_registry = registry.Registry()
> +        
> +    def _cleanup(self):
> +        super(TestLongLogFormatter, self).tearDown()

That last line looks wrong to me. The class ought to be TestCaseWithoutPropsHandler, yes?
And you need to be calling _cleanup() or overriding tearDown(), yes?

The tests look OK though I'd like another one that actually depends on
the revision object being correctly passed to the handler. As best I can
tell, we could be passing any object into the handler and the tests would
still pass.

Ian C.



More information about the bazaar mailing list