[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