----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/114253/#review45147 -----------------------------------------------------------
plasma/generic/dataengines/favicons/faviconprovider.h <http://git.reviewboard.kde.org/r/114253/#comment32263> why is this needed now, if it wasn't needed before? plasma/generic/dataengines/favicons/faviconprovider.h <http://git.reviewboard.kde.org/r/114253/#comment32267> This one is unnecessary for sure. class KJob; would be enough. plasma/generic/dataengines/favicons/faviconprovider.cpp <http://git.reviewboard.kde.org/r/114253/#comment32264> Are you sure that "url" is always a full URL here, and never a local path? KUrl(QString) could deal with local paths, QUrl(QString) can't. It would be much better to change the constructor to take a QUrl argument instead, for type safety. plasma/generic/dataengines/favicons/faviconprovider.cpp <http://git.reviewboard.kde.org/r/114253/#comment32265> missing '/' before favicons -> make it "/favicons/" plasma/generic/dataengines/favicons/faviconprovider.cpp <http://git.reviewboard.kde.org/r/114253/#comment32266> This line is a noop. fromLocalFile is a static method. I think setPath was actually correct here, you're changing the path of an existing full URL. - David Faure On Dec. 3, 2013, 9:53 a.m., Andrea Scarpino wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/114253/ > ----------------------------------------------------------- > > (Updated Dec. 3, 2013, 9:53 a.m.) > > > Review request for KDE Frameworks and Plasma. > > > Repository: kde-workspace > > > Description > ------- > > = subject. > > I replaced KUrl by QUrl and KStandardDirs by QStandardPaths. Please check if > I replaced them correctly. > > > Diffs > ----- > > plasma/generic/dataengines/CMakeLists.txt 3e94325 > plasma/generic/dataengines/favicons/CMakeLists.txt 4af0f14 > plasma/generic/dataengines/favicons/faviconprovider.h 1f370f9 > plasma/generic/dataengines/favicons/faviconprovider.cpp bc72a66 > plasma/generic/dataengines/favicons/favicons.h 79565bf > plasma/generic/dataengines/favicons/favicons.cpp 2eae673 > > Diff: http://git.reviewboard.kde.org/r/114253/diff/ > > > Testing > ------- > > Builds. > > > Thanks, > > Andrea Scarpino > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
