Code review comment for lp://staging/~charlesk/indicator-power/coverage

Thanks for the quick fixes!

On 06/06/2012 10:28 PM, Charles Kerr wrote:
>> the cancellable is passed into all functions and unreffed in dispose()
>> is the cancellable cancelled on unref or do we need to do that explicitely?
>> (I can't find anything in the docs)
>
> This is a great catch. Looking at the gio code, my assumption was wrong,
> it needs to be done explicitly.
>
> Fixed r208

Thanks for checking.

> [...]
>> kind and state should check for sane input values
>
> This is already done by glib's plumbing because the pspec defined the sane range. (No change)

You're right of course. Sorry.

>> 260 ff: I don't think we can use Richard's code (indicators are CA, right?)
>
> Before my refactoring, i-power was already doing this. One difference is now I'm giving proper attribution. (No change, but maybe a ted question?)

Definitely a Ted question.

>> indicator_power_device_new_from_variant: check whether the input variant has the correct type before calling g_variant_get
>
> Fixed r214.

Hm, I was thinking more along the lines of

  g_variant_is_of_type (v, G_VARIANT_TYPE ("(susdut)"));

The fix in r214 only checks for a tuple with the correct number of elements.

>> test/test-dbus-{listener,device}.cc:
>> * ensure_glib_initalized: g_type_init can be called multiple times
>
> I don't think this is correct. Are you sure?

I'm not sure. Actually, I think you might be right.