D25223: Avoid side effects during menu initialization

Konrad Materka noreply at phabricator.kde.org
Tue Nov 12 13:56:23 GMT 2019


kmaterka marked an inline comment as done.
kmaterka added a comment.


  I will send fixes in 5 minutes

INLINE COMMENTS

> broulik wrote in kdeplatformsystemtrayicon.cpp:33
> Is this explicit initialization neccessary?

Not mandatory, works without this. I just wanted to be... explicit. Documentation does not say what is the result of "isNull" when QVariant is not valid.
Remove?

> broulik wrote in kdeplatformsystemtrayicon.cpp:188
> Prefer the specific methods, if exist, `toBool()`

OK

> broulik wrote in kdeplatformsystemtrayicon.h:60
> (This looks like a job for `std::optional`?)

Exactly! But I checked the code of KDE and it is never used. There are even some custom implementations (optional_view, 3 duplicates). QVariant was the closest thing available.

BTW, is KDE officially using C++17?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: kmaterka, #plasma, #frameworks, broulik
Cc: cgiboudeaux, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191112/9918cd34/attachment.html>


More information about the Kde-frameworks-devel mailing list