Review Request: Fix for bug https://bugs.kde.org/show_bug.cgi?id=272098

[Martin Klapetek]

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


Nice work, few comments below. Also, always please use a more descriptive summary, so that when anyone looks, he immediately knows what's this patch for ;)


app/chat-window.h
<http://git.reviewboard.kde.org/r/101352/#comment2755>

    Watch the whitespace here



app/chat-window.cpp
<http://git.reviewboard.kde.org/r/101352/#comment2758>

    Small nitpick - I'd put this method above to the rest of the KStandardActions



app/chat-window.cpp
<http://git.reviewboard.kde.org/r/101352/#comment2757>

    The method itself looks ok, but the placement is a bit off. Always try to put new methods either in the end of the file or where it logicaly seems appropriate (like related methods are one below the other etc). It's a rule of thumb, that first two methods are always a constructor and then a destructor. And you put it in the middle :)


- Martin


On May 13, 2011, 10:12 a.m., Tarun Mall wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101352/
> -----------------------------------------------------------
> 
> (Updated May 13, 2011, 10:12 a.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This is the fix for bug https://bugs.kde.org/show_bug.cgi?id=272098
> 
> 
> This addresses bug https://bugs.kde.org/show_bug.cgi?id=272098.
>     http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=272098
> 
> 
> Diffs
> -----
> 
>   app/chat-window.h 62ccdd5 
>   app/chat-window.cpp a456e44 
> 
> Diff: http://git.reviewboard.kde.org/r/101352/diff
> 
> 
> Testing
> -------
> 
> I tested it with one tab open, more than one tab open.
> Tabs open but not in focus and No tabs are open.
> Working fine with all the test cases.
> 
> 
> Thanks,
> 
> Tarun
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/kde-telepathy/attachments/20110513/5497e34b/attachment-0001.htm