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/plasma-devel/attachments/20191112/9918cd34/attachment.html>
More information about the Plasma-devel
mailing list