[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 #27 from Gyuyoung Kim <gyuyoung.kim at samsung.com>  2012-04-17 02:29:30 PST ---
(From update of attachment 137198)
View in context: https://bugs.webkit.org/attachment.cgi?id=137198&action=review

>>> 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.

ASSERT can work on debug build. Do you only need to check it on debug mode?

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

ditto.

> Source/WebKit/efl/WebCoreSupport/BatteryClientEfl.cpp:100
> +

Remove empty line here as line 85.

>>> 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.

As far as I know, WebKit tends to declare variable when it is really used. So, I think it is better to follow it.

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