[Webkit-unassigned] [Bug 83254] [EFL] Support for Battery Status API on the WebKit-Efl

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=83254





--- Comment #26 from Kihong Kwon <kihong.kwon at samsung.com>  2012-04-16 22:13:21 PST ---
(From update of attachment 137198)
View in context: https://bugs.webkit.org/attachment.cgi?id=137198&action=review

Thank you very much for your review and support. Gyuyoung~;-)

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:48
>> +    ASSERT(e_dbus_init());
> 
> I wonder why do you use ASSERT.

I need to check by assertion to confirm init() is working well.
Because init() is failed, battery status api can't work properly.

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:49
>> +    ASSERT(e_ukit_init());
> 
> ditto.

ditto.

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:116
>> +    bool chargingTimeChanged = false;
> 
> Move dischargingTimeChagned to line 134.

I think all statements of getting battery status and setting variables are a piece of operation.
Otherwise There is no benefit to divide definition and IMHO, this looks better.:-)
How do you think about this?
If you still think I need to move these statements, I will do.

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:117
>> +    bool dischargingTimeChanged = false;
> 
> ditto.

ditto.

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:118
>> +    bool levelChanged = false;
> 
> Move levelChanged to line 156.

ditto.

>> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:122
>> +    double level = 0;
> 
> ditto.

ditto.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.