[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 #28 from Kihong Kwon <kihong.kwon at samsung.com>  2012-04-17 03:24:31 PST ---
(In reply to comment #27)
> (From update of attachment 137198 [details])
> 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?
Yes, every init() call of efl libraries does not check ASSERT.
So, I think there is no needs to check on the release build.
If you want to delete ASSERT, that's no problem to me.

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

OK. I will change their position.

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