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

> > I'm not sure how this fixes bug 1075192.
>
> Well, there are two parts to this bug -- the general case of the sorting, and
> the specific case of the AC Adapter's label. This patch fixes the first part,
> and since this should prevent an AC Adapter from ever being chosen as a
> primary device, the second part won't ever arise.

Got it. Makes me a little uneasy, though ;)

> > (a) according to your explanation in comment 4 on the bug, I should
> experience the same problem -- but I am not able to reproduce this at all
>
> Running i-power 12.10.4 on a laptop with a fully-charged battery should
> trigger the behavior. You can also mockup an equivalent configuration by
> hardwiring it in indicator_power_set_devices():
> [...]

... and it does. I wasn't running 12.10.4 before - sorry about that.

> > (b) a label should *never* be displayed, no matter how the devices are
> sorted
>
> The AC Adapter is the only device where this happens, and with proper sorting
> we'll never choose that device as a primary.
>
> Still, it's a fair point that the code should be correct.
>
> I've added unit tests for this case in r157 and the fix in r158.

Thanks! Makes me feel much less uneasy :)

> > (c) with this fix, the label would still appear when there's only line power
> available (although we don't show the indicator in that case)
>
> This seems to be the same as (b)?

The fix is the same, yes. I was just describing a different scenario when this might have turned up.

Thanks for the fixes!