----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/117112/#review54436 -----------------------------------------------------------
Ship it! (after fixing at least the typo) src/plugin/kpluginloader.h <https://git.reviewboard.kde.org/r/117112/#comment38089> more than ONE plugin, right? I think "one" is missing. src/plugin/kpluginloader.h <https://git.reviewboard.kde.org/r/117112/#comment38090> Ah, you're part of the school of "two spaces after a dot" typography? I thought that was only in old books. src/plugin/kpluginloader.cpp <https://git.reviewboard.kde.org/r/117112/#comment38093> so obviousy I'm not sure it deserves a comment :) - David Faure On March 27, 2014, 12:37 p.m., Alex Merry wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/117112/ > ----------------------------------------------------------- > > (Updated March 27, 2014, 12:37 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kservice > > > Description > ------- > > Make KPLuginLoader encapsulate QPluginLoader, rather than inheriting > > KPluginLoader was overloading various non-virtual methods, which is a > dangerous business. It probably wouldn't have resulted in any bugs that > were *too* serious, other than some possibly mismatched plugin names if > you managed to call setFileName, but it's best to do these things > properly. > > > While SIC, this should not affect anyone who was using KPluginLoader > sensibly, ie: it should only be an issue if you did > QPluginLoader *loader = new KPluginLoader("foo") > > > Diffs > ----- > > src/plugin/kpluginloader.h 56b3e8b9fd8e143fed16be1575c6bf2e5c979285 > src/plugin/kpluginloader.cpp 6d831ac9681e9b26e4644a9e4771eb24c545dac7 > > Diff: https://git.reviewboard.kde.org/r/117112/diff/ > > > Testing > ------- > > Builds, tests pass. I intend to extend the KPluginLoader test, but I thought > I'd get the review up since time is tight before beta1. > > > Thanks, > > Alex Merry > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
