Review Request 123992: Use upcoming version of libdbusmenu-qt

[Sebastian Kügler]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123992/#review81216
-----------------------------------------------------------

Ship it!


5.4 or 5.3.2?

I've noticed that a bunch of smaller things could be improved, though they might well be in the original code already, so can be pushed upstream independently.

I'm OK with shipping this as-is, since this is really the wrong place to fix the fallout I've noticed, so if you feel comfy with it as-is (and will take care of removing the copy again once upstream has merged the changes), then go ahead and ship it.

Most importantly: Fixing crashers is good, and that's why it should go in quickly. Thanks for looking into this.


dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp (line 180)
<https://git.reviewboard.kde.org/r/123992/#comment55609>

    this part would probably benefit from QStringLiteral wrapping, since it looks like a hot path.



dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp (line 220)
<https://git.reviewboard.kde.org/r/123992/#comment55610>

    const (also on the next line)



dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp (line 235)
<https://git.reviewboard.kde.org/r/123992/#comment55611>

    const?



dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp (line 429)
<https://git.reviewboard.kde.org/r/123992/#comment55613>

    compile-time connection would be nice.



dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp (line 466)
<https://git.reviewboard.kde.org/r/123992/#comment55614>

    compile-time slot



dataengines/statusnotifieritem/libdbusmenuqt/dbusmenushortcut_p.cpp (line 37)
<https://git.reviewboard.kde.org/r/123992/#comment55615>

    could be wrapped in more efficient ctors (QL1S, QStringLiteral)


- Sebastian Kügler


On June 3, 2015, 4:03 p.m., David Edmundson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/123992/
> -----------------------------------------------------------
> 
> (Updated June 3, 2015, 4:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This contains a temporary fork of the dbusmenu-qt importer, with the main paths fixed. Also fixes some leaks
> 
> No loner has a stupid event loop. This fixes ~5 crash reports we've had.
> 
> Submitted to upstream, but they're not the fastest and it has to be an API break.
> 
> 
> Diffs
> -----
> 
>   dataengines/statusnotifieritem/CMakeLists.txt e639ee351a1cc6ad0b4448bd0feadc7af0d7984d 
>   dataengines/statusnotifieritem/libdbusmenuqt/README PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.h PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenuimporter.cpp PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenushortcut_p.h PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenushortcut_p.cpp PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.h PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/dbusmenutypes_p.cpp PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/utils.cpp PRE-CREATION 
>   dataengines/statusnotifieritem/libdbusmenuqt/utils_p.h PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/123992/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Edmundson
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20150604/456a5fb8/attachment-0001.html>