Review Request: Fix krazy issues in contact-list

Dario Freddi drf at kde.org
Mon Dec 26 13:41:16 UTC 2011



> On Dec. 26, 2011, 1:16 p.m., Martin Klapetek wrote:
> > context-menu.cpp, line 287
> > <http://git.reviewboard.kde.org/r/103535/diff/1/?file=44843#file44843line287>
> >
> >     According to Qt docs, you should prefer QWeakPointer over QPointer.
> >     
> >     This also btw. does not fix anything as the dialog's data is never accessed, but if it makes Krazy happy, then so be it.
> 
> Vishesh Handa wrote:
>     How does this help in anyway? If anything, it makes the code harder to understand, and (in the event that your care about exceptions) less exception-safe.
> 
> Dominik Cermak wrote:
>     That's were krazy links to: http://blogs.kde.org/node/3919

QPointer is deprecated for this kind of issues and should never be used - please follow Martin's advice and replace it with QWeakPointer.


> On Dec. 26, 2011, 1:16 p.m., Martin Klapetek wrote:
> > global-presence-chooser.cpp, line 236
> > <http://git.reviewboard.kde.org/r/103535/diff/1/?file=44845#file44845line236>
> >
> >     Have you tested this? I'm not sure if KComboBox does not reimplement this method and does some additional magic that could break this.
> 
> Dominik Cermak wrote:
>     It doesn't re-implement both of them. Anyway it brings no real benefit beside making krazy happy...
>     So change it or leave it?

This should not affect the code itself. Although, QComboBox kinda makes the code clearer, at least to me, so I'd leave the Q one here. Remember, in these cases you can suppress krazy warnings like this: http://techbase.kde.org/Development/Tutorials/Code_Checking#Suppressing_false-positives


- Dario


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


On Dec. 26, 2011, 1:10 p.m., Dominik Cermak wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/103535/
> -----------------------------------------------------------
> 
> (Updated Dec. 26, 2011, 1:10 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Add KUIT context marker.
> Normalize signatures.
> Fix typos.
> Use K-classes.
> Use QPointers when showing modal dialogs via exec().
> Fix null string assign.
> Fix newline.
> 
> 
> Diffs
> -----
> 
>   account-button.cpp 1bcc93c4f75c77f45be91723f53787660e6a6ae0 
>   contact-delegate-overlay.cpp 2cae71f8e9a96e8cd6dce6bc1ac782937bd1d8bc 
>   context-menu.cpp e98392ea0ec73b89fc50bff64b8a0866177e551e 
>   dialogs/join-chat-room-dialog.ui 4e90ebf407dbf0c0005f92d8ec8f18962f91dfdc 
>   global-presence-chooser.cpp 935f58266bfdad801d98c8c93eafe3515669fd52 
>   tooltips/contacttooltip.cpp 46981fa296ee22bedab9bb9cc385f2cc054f06a6 
> 
> Diff: http://git.reviewboard.kde.org/r/103535/diff/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dominik Cermak
> 
>

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


More information about the KDE-Telepathy mailing list