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

Guillermo Gonzalez guillo.gonzo at gmail.com
Fri Jul 4 20:18:53 BST 2008


Hi Jelmer

On Fri, Jul 4, 2008 at 7:45 AM, Jelmer Vernooij <jelmer at samba.org> wrote:
> Hi Guillermo,
>
> Am Freitag, den 04.07.2008, 11:12 +0200 schrieb Jelmer Vernooij:
>> This merge request appears to be dead - do you still intend to work on
>> it?

yes, I still want to work on this and try to merge it into the core.

>
> I played with your patch a little bit to fix
> https://launchpad.net/bugs/161830. Would it be possible to pass the full
> revision object rather than just its properties member? bzr-svn would in
> particular need the revision id so it can parse that and do sqlite
> lookups based on it.

Absolutely, :-)
I can spend some time on this during this weekend, basically I need to
rework the error handling bit (any ideas are welcome ;-) )

Cheers,

--
Guillermo


>> Am Montag, den 31.03.2008, 23:17 -0300 schrieb Guillermo Gonzalez:
>> > On Mon, Mar 31, 2008 at 10:09 AM, Aaron Bentley <aaron at aaronbentley.com> wrote:
>> > [snip]
>> > Aaron,
>> >    First of all, thanks for the extensive review :)
>> > >
>> > >  > +    def show_properties(self, properties):
>> > >  > +        """Displays the custom properties returned by each registered handler.
>> > >  > +
>> > >  > +        If a registered handler raise an error, it's silently logged and the
>> > >  > +        next handler in the registry is executed.
>> > >  > +        """
>> > >
>> > >  Shouldn't that be considered a bug in the handler?  What kinds of errors
>> > >  are you saying should be ignored?
>> > >
>> >
>> > I agree that it should be considered a bug, but I wasn't sure if
>> > propagating the error and stop the log for a broken handler or keep
>> > executing log.
>> >
>> > should the log stop if a handler is broken?
>> >
>> > >  Also, your docstring should have "raises", not "raise".
>> >
>> > apologize me for the typo, I'll fix it.
>> >
>> > >
>> > >  > +            try:
>> > >  > +                for key, value in handler(filtered_props).items():
>> > >  > +                    self.to_file.write(key + ': ' + value + '\n')
>> > >  > +            except:
>> > >  > +                mutter('custom property handler: %s raised an error',
>> > >  > +                        key)
>> > >  > +                trace.log_exception_quietly()
>> > >
>> > >  Bare except clauses are almost always a bad idea.  For instance, this
>> > >  will ignore a KeyboardInterrupt or a pipe error.
>> >
>> > Thanks for the tip.
>> > How should it handle this errors? (I don't know exactly what errors a
>> > handler can raise, actually it can be any python or bzr related error)
>> >
>> > >
>> > >  > === modified file 'bzrlib/tests/blackbox/test_log.py'
>> > >  > --- bzrlib/tests/blackbox/test_log.py 2008-01-10 22:34:09 +0000
>> > >  > +++ bzrlib/tests/blackbox/test_log.py 2008-03-31 03:56:49 +0000
>> > >  > @@ -184,6 +184,56 @@
>> > >  >          self.assertNotContainsRe(log, r'revno: 1\n')
>> > >  >          self.assertContainsRe(log, r'revno: 2\n')
>> > >  >          self.assertContainsRe(log, r'revno: 3\n')
>> > >  > +
>> > >
>> > >  These don't look like UI tests to me.  They should probably be in
>> > >  bzrlib/tests/test_log, instead.
>> >
>> > I didn't added those three asserts, but I don't have any problem to move them.
>> >
>> > >
>> > >  > +    def test_log_without_custom_properties_handler(self):
>> > >  > +        tree = self._prepare()
>> > >  > +        tree.commit(message='', revprops={'first_prop':'first_value'})
>> > >  > +        log = self.run_bzr("log --limit 1")[0]
>> > >  > +        self.assertNotContainsRe(log, r'first_prop: first_value\n')
>> > >  > +        log = self.run_bzr("log -r1")[0]
>> > >  > +        self.assertNotContainsRe(log, r'first_prop: first_value\n')
>> > >
>> > >  ^^^ How does this ensure that there are no custom properties handlers?
>> > >
>> >
>> > ups, I missed that one. I keep forgetting that selftest load plugins.
>> > the test assume the registry is empty (my mistake), but it should
>> > clean the registry or replace the registry with an empty one.
>> >
>> > I'll work on this issues and resubmit.
>> >
>> > Thanks for the comments and guidance.
>> >
>> > Cheers,
>
> --
> Jelmer Vernooij <jelmer at samba.org> - http://samba.org/~jelmer/
> Jabber: jelmer at jabber.fsfe.org
>



More information about the bazaar mailing list