Code review comment for lp:~canonical-platform-qa/autopilot/cpo-base-classes-fix

Hi Chris, yes, I totally agree that adding the MRO stuff to the combine method is changing its initial behaviour. I added it after trying to make the current tests pass, I've commented in the code trying to make it clear.

Using the combine method to put together arbitrary tuples was giving at first an error about a cyclic dependency, one of the elements of the tuple was the class itself.

  TypeError: a __bases__ item causes an inheritance cycle

This can be solved by passing the original class itself to the method, and checking that it is not a member of the collections being merged. At this point, we could continue treating the combine method as an abstract information merger. But after this, a second error arose:

  TypeError: Cannot create a consistent method resolution order (MRO)

So that, to combine them appropriately, the method has to know which is the right order, and this information can be obtained from the classes MROs.

Unless I'm missing something it seems that the combine method has to know that it is combining classes, moreover it needs to know that the return value is going to be the bases tuple of one of the parameters. IMO the responsibility is well delimited, maybe we could split it.

Anyway, I'll try to implement the tests that you mention, thanks a lot Chris!