Code review comment for lp:~gz/bzr/transport_post_connect_hook

Revision history for this message
Vincent Ladeuil (vila) wrote :

129 + for hook in self.hooks["post_connect"]:
130 + # GZ 2010-10-14: Should the hook be passed the new connection and
131 + # credentials too or does opaque really mean that?
132 + hook(self)

The hook already receives 'self' so it can access the connection/credentials if needed.
But they are specific to each transport class...

The tests are ok. We know the hook is heavily exercised or we get leaks anyway.

If you really really want to add tests you can check what happens when several transports share a connection, but even that sounds overkill.

Write the NEWS entry, we'll see what is available when your patch lands. Don't forget the hooks-help.txt file.

I'll ping people on the pre-requisites.

review: Approve

« Back to merge proposal