Code review comment for lp:~vila/bzr/389648-hook-calls-base

As mentioned above, the target was wrong, it should be 'lp:bzr' which is ~bzr-pqm/bzr/bzr.dev, not ~bzr/bzr/trunk.

Anyway, I did the review locally. It was still a bit hard to follow, because a lot of 'formatting' updates. Stuff like:

-class TestHook(TestCase):
+class TestHook(tests.TestCase):

I wonder if this wouldn't be like the whitespace fixes, and we should have "one big patch" across the codebase to fix them, rather than getting them piecemeal. Then again, the same arguments apply for and against that were present for the whitespace updates...

Anyway, I think the basic fix here was:

1) Add
+ super(InfoHooks, self).__init__()

To a couple of the hook instances

2) Add
+ self.assertEqual("No hook name", new_hooks.get_hook_name(None))
To the tests to ensure that the internal dictionary is created and able to return a value for an unnamed hook.

Which seems just fine.

I should comment that using "super()" is not always correct anymore. To start with, it only matters when you have multiple inheritance. But even more importantly, python 2.6 'broke' super(...).__init__() if there are any arguments. Because object.__init__() no longer allows arguments. (So as near as I can tell, there is no way to safely __init__ a multiple inheritance structure that wants arguments passed as part of init. Unless maybe you do something like use a single abstract base class that all classes inherit from, that *doesn't* call super?)

Anyway, IMO super().__init__() was broken in 2.6, though certainly someone thought it was the right thing to do.