[Differential] [Commented On] D1647: kfontinst: Port to QDialog

apol (Aleix Pol Gonzalez) noreply at phabricator.kde.org
Fri May 20 13:40:24 UTC 2016


apol added a comment.


  Other than that it looks quite good, interesting how much code gets cleaned up.

INLINE COMMENTS

> Printer.cpp:349
> +
> +    QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Cancel);
> +    connect(buttonBox, SIGNAL(rejected()), this, SLOT(slotCancelClicked()));

Pass this as the last argument, so it has a parent and doesn't leak. On this one and the rest of `new QDialogButtonBox`.

> Printer.cpp:350
> +    QDialogButtonBox *buttonBox = new QDialogButtonBox(QDialogButtonBox::Cancel);
> +    connect(buttonBox, SIGNAL(rejected()), this, SLOT(slotCancelClicked()));
> +

Use new syntax. `&QDialog::rejected` and `&CPrinter::slotCancelClicked`

> DuplicatesDialog.cpp:76
> +    itsButtonBox = new QDialogButtonBox(QDialogButtonBox::Cancel);
> +    connect(itsButtonBox, SIGNAL(clicked(QAbstractButton*)), this, SLOT(slotButtonClicked(QAbstractButton*)));
> +    QVBoxLayout *mainLayout = new QVBoxLayout;

use new syntax as well.

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D1647

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: volkov, Plasma
Cc: apol, plasma-devel, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20160520/cca24413/attachment-0001.html>


More information about the Plasma-devel mailing list