Review Request 113821: Don't install kpagedialog_p.h

[David Edmundson]

> On Nov. 13, 2013, 11:53 a.m., David Edmundson wrote:
> > tier4/kcmutils/src/kcmultidialog.cpp, line 262
> > <http://git.reviewboard.kde.org/r/113821/diff/1/?file=213192#file213192line262>
> >
> >     I don't think this is right.
> >     
> >     If we copy a KCMultiDialog instance, we wouldn't copy the KPageDialog d variable of the parent object, but instead create a new one.
> >     
> >     I think this would be a behavioural change.
> 
> Aleix Pol Gonzalez wrote:
>     Well, if I'm not mistaken, it's the same that KPageDialog was doing already:
>     
>     KPageDialog::KPageDialog(KPageDialogPrivate &dd, KPageWidget *widget, QWidget *parent, Qt::WindowFlags flags)
>         : QDialog(parent, flags),
>           d_ptr(&dd)
>

Previously your d pointer inherited from KPageDialog's d pointer. There was one instance of the private object, which KPageDialog copied.

Now you have two instances of private objects and two d pointers. One in KMultiDialog, one in the superclass, KPageDialog. You copy the KMultiDialogPrivate object, but the private class in KPageDialog is not copied, as we never call the copy constructor in the superclass.


- David


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


On Nov. 12, 2013, 6:46 p.m., Aleix Pol Gonzalez wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113821/
> -----------------------------------------------------------
> 
> (Updated Nov. 12, 2013, 6:46 p.m.)
> 
> 
> Review request for KDE Frameworks and David Faure.
> 
> 
> Repository: kdelibs
> 
> 
> Description
> -------
> 
> This makes sure that KWidgetsAddons doesn't expose any private dependencies.
> 
> The only user of that file was KCMultiDialogPrivate. This patch refactors the code so that it's used separately.
> 
> 
> Diffs
> -----
> 
>   tier1/kwidgetsaddons/src/CMakeLists.txt 76679a4 
>   tier4/kcmutils/src/kcmultidialog.h 3518736 
>   tier4/kcmutils/src/kcmultidialog.cpp 9d2ccbb 
>   tier4/kcmutils/src/kcmultidialog_p.h ad5dd98 
> 
> Diff: http://git.reviewboard.kde.org/r/113821/diff/
> 
> 
> Testing
> -------
> 
> Everything builds, couldn't test since I didn't see any test.
> 
> Tests still pass though.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20131113/24fbb269/attachment.html>