D4939: Get rid of KDELibs4Support

Alexander Volkov noreply at phabricator.kde.org
Tue Mar 7 10:36:55 UTC 2017


volkov added inline comments.

INLINE COMMENTS

> CMakeLists.txt:111
> +qt5_add_dbus_adaptor(powerdevil_SRCS ${SOLID_DBUS_INTERFACES_DIR}/kf5_org.freedesktop.PowerManagement.xml powerdevilfdoconnector.h PowerDevil::FdoConnector powermanagementfdoadaptor PowerManagementFdoAdaptor)
> +qt5_add_dbus_adaptor(powerdevil_SRCS ${SOLID_DBUS_INTERFACES_DIR}/kf5_org.freedesktop.PowerManagement.Inhibit.xml powerdevilfdoconnector.h PowerDevil::FdoConnector powermanagementinhibitadaptor PowerManagementInhibitAdaptor)
>  

I guess it can be just removed. These kf5_org.* files were dropped from solid three years ago.

> CMakeLists.txt:22
>  add_powerdevil_bundled_action(runscript KF5::KIOCore KF5::KIOWidgets)
> -add_powerdevil_bundled_action(suspendsession KF5::KIOCore KF5::KIOWidgets KF5::Solid KF5::KDELibs4Support)
> +add_powerdevil_bundled_action(suspendsession KF5::KIOCore KF5::KIOWidgets KF5::Solid)
>  if(HAVE_WIRELESS_SUPPORT)

Is it necessary to add KF5::Solid?

> powerdevilcore.cpp:102
> +{
> +    if (!m_backend)
> +        return BackendInterface::UnknownSuspendMethod;

And who will set backend for Core?

Now I think that it was a bad idea to request supported suspend methods from a backend, 
because GUI doesn't depend on it. You need to load the backend in GUI, it will do some initialization...

It seems to be more reasonable to request supported methods from powerdevil daemon by dbus calllings
org.freedesktop.PowerManagement.{CanSuspend, CanHibernate, CanHybridSuspend}

> powerdevilcore.cpp:107
> +
> +void Core::generateDefaultProfiles()
> +{

unrelated

> activitypage.cpp:76
>      // Message widget
> -    m_messageWidget = new KMessageWidget(i18n("The activity service is running with bare functionalities.\n"
> -                                                          "Names and icons of the activities might not be available."));
> +    m_messageWidget = QSharedPointer<KMessageWidget>(new KMessageWidget(i18n("The activity service is running with bare functionalities.\n"
> +                                                                             "Names and icons of the activities might not be available.")));

Shouldn't it be in a separate change?

REPOSITORY
  R122 Powerdevil

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

To: denisshienkov, volkov, afiestas
Cc: graesslin, davidedmundson, broulik, plasma-devel, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170307/f2145589/attachment-0001.html>


More information about the Plasma-devel mailing list