Review Request 109262: busyOverlay now hides if presence chooser is editable.

[Roman Nazarenko]

> On March 3, 2013, 5:05 p.m., Martin Klapetek wrote:
> > global-presence-chooser.cpp, lines 305-307
> > <http://git.reviewboard.kde.org/r/109262/diff/2/?file=116716#file116716line305>
> >
> >     This needs to be called only when editable is false; also please add a comment that we're restarting the spinner if it was spinning before

If it's false, overlay's widget will be set to NULL, and start() function will exit anyway - it checks widget==null in the first line, so it's not a really a big deal. 
I leaved it so because adding if (editable) will add us third nesting level, what leads to poor readability. May add a comment about it in the sources though.


> On March 3, 2013, 5:05 p.m., Martin Klapetek wrote:
> > global-presence-chooser.cpp, line 303
> > <http://git.reviewboard.kde.org/r/109262/diff/2/?file=116716#file116716line303>
> >
> >     You don't need this check as this method won't be ever called before the constructor, which constructs this widget. But won't hurt either.

It was crashing once because of that check miss, so we'd rather leave it so. CPUs are optimizing those logical forkings good enough for us to leave those things.


- Roman


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/109262/#review28461
-----------------------------------------------------------


On March 3, 2013, 4:55 p.m., Roman Nazarenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109262/
> -----------------------------------------------------------
> 
> (Updated March 3, 2013, 4:55 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> busyOverlay now hides if presence chooser is editable.
> 
> 
> This addresses bug 292282.
>     http://bugs.kde.org/show_bug.cgi?id=292282
> 
> 
> Diffs
> -----
> 
>   global-presence-chooser.h d7a19c4 
>   global-presence-chooser.cpp 9e7994f 
> 
> Diff: http://git.reviewboard.kde.org/r/109262/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Roman Nazarenko
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130303/a087d250/attachment.html>