----------------------------------------------------------- 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 > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
