Code review comment for lp:~didrocks/autopilot/remove-circular-dep

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

Guys,

As I'm sure you're aware, developers can become a little too posessive over their code. Perhaps this is one such instance, but please don't push MPs through to land without consulting the relevant people for a review first.

MTerry and I discussed this in Oakland. The python-autopilot package is useless without *either* autopilot-touch *or* autopilot-desktop installed. Literally useless. You won't be able to create any input device, or get any display information, or use any of the other autopilot drivers.

We (mterry and I) did some research regarding circular dependencies, and (if my memory serves me right) we found that in the case of circular dependencies, apt will break the chain at an arbitrary point. This is only a problem if you have (post|pre)(inst|rm) scripts that require your dependency to be installed at the time the script is run. We do not fit this category. The only script we run is to make the uinput-autopilot device node, which does not depend on the autopilot-touch or autopilot-desktop code.

So, if you'd like this merged, please explain why this change is a good thing. What problem does this MP solve? How will you solve the problem of people installing python-autopilot only, and then getting a broken autopilot setup?

I hope this doesn't come across too harsh - I really don't mean it to, but it's an unpleasant surprise to see things like this being merged when I was not made aware of any issue that needed to be solved.

Cheers,

review: Needs Information

« Back to merge proposal