Review Request: ChatUi: Refactor

George Kiagiadakis kiagiadakis.george at gmail.com
Fri Feb 4 17:05:48 CET 2011


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



app/chatwindow.h
<http://git.reviewboard.kde.org/r/100547/#comment1030>

    This is a local include. It should be using "" and it should be above the others. Even if this is in the lib directory, it should be treated like a local include, or else the compiler will look in the standard path first.



app/main.cpp
<http://git.reviewboard.kde.org/r/100547/#comment1025>

    Watch out, you changed the license here!!!! And what's more, GPL3 is not ok for inclusion in kde, so if this is ok with you, please change back to GPL2. This applies for all the other files in this commit too.
    
    (PS: There are also other files in the chat-handler repository that are GPL3. It would be good to check them all and change them to GPL 2)



app/main.cpp
<http://git.reviewboard.kde.org/r/100547/#comment1028>

    It's a good idea to include local includes before global includes. See http://techbase.kde.org/Policies/Library_Code_Policy#Getting_.23includes_right



app/main.cpp
<http://git.reviewboard.kde.org/r/100547/#comment1029>

    This is a leak. This class is never deleted. It would be better to allocate it on the stack, as the KApplication previously was.



app/telepathychatui.h
<http://git.reviewboard.kde.org/r/100547/#comment1027>

    The same here (local includes first).



app/telepathychatui.cpp
<http://git.reviewboard.kde.org/r/100547/#comment1026>

    The same here (local includes first).


- George


On Feb. 4, 2011, 12:43 p.m., Dominik Schmidt wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/100547/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2011, 12:43 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> Use a proper KXmlGuiWindow and flexible structure.
> 
> 
> Diffs
> -----
> 
>   app/CMakeLists.txt 86bea23 
>   app/chatwindow.h PRE-CREATION 
>   app/chatwindow.cpp PRE-CREATION 
>   app/chatwindow.rc PRE-CREATION 
>   app/main.cpp 9ef06f9 
>   app/mainwindow.h badff27 
>   app/mainwindow.cpp 54ac071 
>   app/telepathychatui.h PRE-CREATION 
>   app/telepathychatui.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/100547/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dominik
> 
>

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


More information about the KDE-Telepathy mailing list