Code review comment for lp://staging/~diegosarmentero/ubuntu-system-settings/update-battery

Revision history for this message
Sebastien Bacher (seb128) wrote :

Thanks you for your work, some issues

- you use "label.text" there, but there is no such object (you get a runtime warning about it being undefined btw)

- I just tried on my laptop (using the "sudo system-image-dbus --testing=update-manual-success" mock) which is plugged and 100% charged, it gave me the error case/not now interface

My case on that one is th at "onRemainingCapacityChanged" is never called on my fully charged scenario, so the batterySafeForUpdate is never set

- the 50% charge seems a bit much, but the spec doesn't define what would be a reasonable level, I guess 30% should be enough ... I'm not going to be picky about that though

- clicking on "not now" brought me back to the updates list, clicking on the "install" button again made the install happen despite the incorrect charge/without asking

(that one is not a new bug I think though)

review: Needs Fixing

« Back to merge proposal