Review Request 109142: KDE global menu support over DBus added

[David Edmundson]

> On Feb. 25, 2013, 4:19 p.m., Dan Vrátil wrote:
> > main-widget.h, line 97
> > <http://git.reviewboard.kde.org/r/109142/diff/1/?file=115337#file115337line97>
> >
> >     The variables should be all prefixed with "m_" to distinguish them from local variables.
> >     
> >     You should also set all the pointer to 0 in constructor.
> 
> Roman Nazarenko wrote:
>     Why should I? All my new pointers are initialized during construction, there's no need in null-initialization, I guess.

It's for safety. When you start programming in a large open source project you have to think about "which idiot is going to alter my code in two years from now?". (probably me)

You're absolutely right it's not "needed", however when someone in the future then alters your code and accidentally tries using the m_globalMenu early in the constructor they get a crash that's really hard to debug. A null pointer is pretty obvious, a crash deep inside KAction as you have a pointer to random data would take a much longer time to debug what's wrong.


- David


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


On Feb. 25, 2013, 5:23 p.m., Roman Nazarenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/109142/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2013, 5:23 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Added global menu support.
> Menu is not appearing in main window, so it's tiny look is not broken.
> I also moved actions for menu from MainWidget constructor to class private scope, so they can be used from any method. 
> 
> 
> This addresses bug https://bugs.kde.org/show_bug.cgi?id=292396.
>     http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=292396
> 
> 
> Diffs
> -----
> 
>   main-widget.h d14a669 
>   main-widget.cpp a3ea666 
> 
> Diff: http://git.reviewboard.kde.org/r/109142/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Roman Nazarenko
> 
>

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