dfaure added a comment.
I like the encapsulation into a different class.
INLINE COMMENTS
> dropjob.cpp:63
> +public:
> + DropMenu(QWidget *parent = 0);
> + ~DropMenu();
nullptr
> dropjob.cpp:167
> +
> +void DropMenu::addExtraActions(QList<QAction *> appActions, QList<QAction *>
> pluginActions)
> +{
unnecessary copying (refcounting), the two QLists should be passed as const ref.
> dropjob.cpp:173
> + removeAction(m_extraActionsSeparator);
> + for (QAction *action : m_appActions) {
> + removeAction(action);
(which would avoid the detaching here)
> dropjob.cpp:350
> popup->addSeparator();
> - popup->addAction(popupCancelAction);
> +// popup->addAction(popupCancelAction);
> }
please clean up
> dropjob.h:38
> class DropJobPrivate;
> +class DropMenu;
>
I don't think this is needed, nothing else in the header refers to it (I guess
it was an initial solution with private slots that you then moved to the
private class)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D4739
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: mart, #plasma, #frameworks, dfaure
Cc: plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts,
sebas, apol
