Code review comment for lp:~gordallott/unity/hud

Gord Allott (gordallott) wrote :

> Hi,
> There are a few introspection / autopilot issues with this branch:
> Your HUDController derives from Introspectable, but you never use it. You need
> to:
> 1) Add it to the introspection tree.At Diff line 2729 (where you create the
> HUDController instance) you need to call "AddChild(hud_controller_.get());".
> If the HUDController is ever deleted, you need to call
> "RemoveChild(hud_controller_.get());" BEFFORE the deletion takes place.
> 2) Add properties to the HUD controller that will be useful in autopilot
> tests.
> You need to repeat this process with all child objects of the HUDController
> that derive from Introspectable.
> Finally, the string returned from "GetName" should be "HUDController" - you
> don't need to qualify it with "unity.hud".
> In HUDIcon.h you #include "Introspectable.h", but never use it.
> In HUDView.h, you've declared a "const gchar* GetName()" method, which is from
> Introspectable, but there's two problems:
> 1) You don't derive from Introsopectable (as noted above, you should).
> 2) That's not the correct method signiture - the return type is
> "std::string", and the method is const.
> In the same class, make sure you add the search bar to the introspection tree
> - that way we can get the current search string.

Very much so, Introspection was on my todo list for earlier in the week before last minute changes bumped it off, will fix that up

> Finally, it'd be really nice to have some autopilot tests written when this is
> merged. I don't mind helping you with this.

Already have a guy from Systems helping out there, for now manual but he's going through and converting them :)

> Cheers,

« Back to merge proposal