----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/119382/#review63168 -----------------------------------------------------------
CMakeLists.txt <https://git.reviewboard.kde.org/r/119382/#comment43929> QT_MIN_VERSION seems to be unintialized (empty) here CMakeLists.txt <https://git.reviewboard.kde.org/r/119382/#comment43930> maybe use set(CMAKE_INCLUDE_CURRENT_DIR ON) instead? devicecache.cpp <https://git.reviewboard.kde.org/r/119382/#comment43932> kError -> qCDebug? Shouldn't this be qCWarning or qCCritical? It seems to me that you can't use the device at all if that happens, and therefore it should be more than a debug message. Same thing in next case statement. devicecache.cpp <https://git.reviewboard.kde.org/r/119382/#comment43933> Same thing again (kError -> qCDebug) kio_mtp.h <https://git.reviewboard.kde.org/r/119382/#comment43934> Use Q_DECL_OVERRIDE after the function instead of (or in addition to) virtual here and in the following lines kio_mtp.cpp <https://git.reviewboard.kde.org/r/119382/#comment43936> I don't think you need the extra 1 line function here, just write the `url.adjusted(QUrl::StripTrailingSlash).path()` inline. Or if you keep the extra function and rename it to something else. With the current name I would expect it to return the full url without trailing slash, not just the path. As far as I can see it is only used once, so not much reason to keep it. - Alexander Richardson On Juli 21, 2014, 1:54 nachm., Jan Grulich wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/119382/ > ----------------------------------------------------------- > > (Updated Juli 21, 2014, 1:54 nachm.) > > > Review request for KDE Frameworks and Philipp Schmidt. > > > Repository: kio-mtp > > > Description > ------- > > I ported kio-mtp to KF5 including: > kDebug -> qCDebug > KUrl -> QUrl > > And it doesn't need Kdelibs4Support. > > Also one thing which should be probably considered is including kio-mtp in > kio-extras. > > > Diffs > ----- > > kio_mtp_helpers.cpp 0f9ddbc > CMakeLists.txt cb27440 > devicecache.h 1fa3932 > devicecache.cpp 54ca56f > filecache.h 590e799 > filecache.cpp dfd4fe6 > kio_mtp.h b05f09b > kio_mtp.cpp 6e587fa > kio_mtp_helpers.h b66adbd > > Diff: https://git.reviewboard.kde.org/r/119382/diff/ > > > Testing > ------- > > My phone is visible in Dolphin and I was able to finally copy my photos from > phone to my laptop. > > > Thanks, > > Jan Grulich > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
