Review Request: Create an option for the behaviour of new tabs

[David Edmundson]

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


Just to explain the d-pointer thing:

This is an excellent guide on why it's done
http://zchydem.enume.net/2010/01/19/qt-howto-private-classes-and-d-pointers/

We do it in libraries to maintain binary comparability, so if someone else makes an application which uses our lib (for example KBattleship includes the ChatWidget) it won't break when we want to change the lib.
Without d-pointers if we added a private member it would change the size of our class. So any application compiled against an old version of the lib expects it to be one size, the library would have something else. It all crashes and the world explodes. We don't want that. With d pointers, only the size of a private class changes, which we can do.

For the KCM, even though it does install like a library, no-one links against it, so this whole situation doesn't matter.

summary:
 is some-one else linking to our code? 
   yes : use d-pointers.
   no  : personal preference, you don't need to.

- David


On July 8, 2011, 7:10 p.m., Lasath Fernando wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/101886/
> -----------------------------------------------------------
> 
> (Updated July 8, 2011, 7:10 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Summary
> -------
> 
> This was rather a big refractor (for me at least), and this is the first time I created any new classes. So I'm not sure I did quite a few things right.
> 
> I created a new tab in the KCM, called Behaviour. I think we'll probably have to change the labels on the radiobuttons to something that makes more sense. 
> 
> I also included "../app/telepathy-chat-ui.h" because I needed an enum from it. I don't know if there's anything against doing that or not.
> 
> I also am not sure I used a QButtonGroup correctly. 
> 
> Also, I noticed the other class didn't use a d-pointer, (even though a KCM is supposed to be like a library file) so I didn't. 
> 
> There are many more things I was doubtful at the time, but I can't possibly remember now (I shouldn't leave writing review requests till 2AM). Hopefully, everything makes sense in the diff.
> 
> And as always, my code is in my scratch repo:
> http://quickgit.kde.org/?p=clones%2Ftelepathy-chat-handler%2Ffernando%2FdetachableTabs.git&a=shortlog&h=refs/heads/refractored_tabs
> 
> 
> Diffs
> -----
> 
>   app/telepathy-chat-ui.h ad6809f 
>   app/telepathy-chat-ui.cpp 71ac4ee 
>   config/CMakeLists.txt 48252a7 
>   config/appearance-config.h PRE-CREATION 
>   config/appearance-config.cpp PRE-CREATION 
>   config/behavior-config.h PRE-CREATION 
>   config/behavior-config.cpp PRE-CREATION 
>   config/behaviorconfig.ui PRE-CREATION 
>   config/main-window.h e95bf93 
>   config/main-window.cpp 3a79da4 
>   config/main.cpp c47f493 
> 
> Diff: http://git.reviewboard.kde.org/r/101886/diff
> 
> 
> Testing
> -------
> 
> Changed a (well, the only) setting, and the program changed behaviour as expected. 
> 
> 
> Thanks,
> 
> Lasath
> 
>

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