mwolff requested changes to this revision.
mwolff added a comment.
This revision now requires changes to proceed.
the UDI comment needs to be investigated, otherwise lgtm
INLINE COMMENTS
> kfileplacesmodeltest.cpp:181
> return QStringList() << QDir::homePath() << QStringLiteral("remote:/")
> << QStringLiteral(KDE_ROOT_PATH) << QStringLiteral("trash:/")
> - << QStringLiteral("/media/nfs") <<
> QStringLiteral("/media/floppy0") << QStringLiteral("/media/XO-Y4") <<
> QStringLiteral("/media/cdrom") << QStringLiteral("/foreign");
> + << QStringLiteral("/media/nfs") <<
> QStringLiteral("/foreign")
> + << QStringLiteral("/media/floppy0") <<
> QStringLiteral("/media/XO-Y4") << QStringLiteral("/media/cdrom");
the changed list of remote urls that is repeated below could be put into
another helper function
> kfileplacesitem.cpp:91
> + if (m_device.udi().isEmpty()) {
> + updateDeviceInfo(m_bookmark.metaDataItem(QStringLiteral("UDI")));
> + }
shouldn't this always be called? i.e. when the bookmark is changed to a
different UDI, don't we need to update here, even when we had a valid device
UDI before? I think this also means this path isn't unit tested yet
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D8348
To: renatoo, #dolphin, #frameworks, #vdg, ervin, ngraham, mwolff
Cc: mwolff, abetts, mlaurent, anthonyfieroni, ngraham, #frameworks
