[MERGE][bug 128076][bug 131396][0.91] Reuse bound branch transport to avoid multiple connections

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Sep 11 06:29:47 BST 2007


>>>>> "martin" == Martin Pool <mbp at canonical.com> writes:

    martin> Martin Pool has voted tweak.
    martin> Status is now: Conditionally approved
    martin> Comment:

    martin> +        self.install_hooks()
    martin> +
    martin> +        update = builtins.cmd_update()
    martin> +        update.run()
    martin> +        self.assertEquals(1, len(self.connections))
    martin> +

    martin> (comment)
    martin> It's nice that we can actually test this connects only once.  It
    martin> would be
    martin> good to add some text to HACKING describing how people can use
    martin> TestCaseWithConnectionHookedTransport to test this kind of thing.

I want to make other modifications in that area and will include
that.

    martin> Also in that class it would have been better to call
    martin> it something like start_logging_connections(), rather
    martin> than the more general install_hooks().

Yeah... I love reviews :)

    martin> -    def get_master_branch(self):
    martin> +    def get_master_branch(self, possible_transport=None):

    martin> (tweak) Why singular 'possible_transport' not plural?  I guess
    martin> it's just a typo?

Eerk ! Yes, fortunately it was in the mother class, so not called :-/

Yet another Freudian slip, I was unsure about modifying the
signature here and was seeking review comments :)

Only Branch5 really handles bound branches, but one day Branch4
will be deprecated anyway...

    martin> Aside from that looks good for 0.91.

Ok.

        Vincent



More information about the bazaar mailing list