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
