D10414: Add move semantics support to KIO::UDSEntry.

[David Faure]

dfaure added inline comments.

INLINE COMMENTS

> udsentrytest.cpp:227
> +/**
> + * Test to verify that move semantics work. This is only useful when ran through callgrind.
> + */

... or gdb, or perf, or with debug output in the cpp file, or ....

> udsentrytest.cpp:234
> +    QVERIFY(file.open());
> +    const QByteArray filePath = file.fileName().toLocal8Bit();
> +    const QString fileName = QUrl(file.fileName()).fileName(); // QTemporaryFile::fileName returns the full path.

QFile::encodeName() is the proper way

> udsentrytest.cpp:235
> +    const QByteArray filePath = file.fileName().toLocal8Bit();
> +    const QString fileName = QUrl(file.fileName()).fileName(); // QTemporaryFile::fileName returns the full path.
> +    QVERIFY(!fileName.isEmpty());

That's rather overkill (and a wrong use of the QUrl API). You want to use QFileInfo for this.

> udsentrytest.cpp:240
> +    QT_STATBUF statBuf;
> +    QVERIFY(QT_LSTAT(filePath.constData(), &statBuf) == 0);
> +    KIO::UDSEntry entry(statBuf, fileName);

Whenever you write QVERIFY(a==b), it should be QCOMPARE(a, b)

> udsentrytest.cpp:256
> +
> +        // And veryfy that this works.
> +        QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME));

typo: verify

> udsentrytest.cpp:268
> +
> +        // And veryfy that this works.
> +        QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME));

same typo

> udsentrytest.cpp:269
> +        // And veryfy that this works.
> +        QCOMPARE(fileName, movedEntry.stringValue(KIO::UDSEntry::UDS_NAME));
> +    }

swap it around: what you test on the left, what you expect on the right. This makes error messages more readable.  (same above, of course)

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D10414

To: markg, dfaure
Cc: apol, #frameworks, michaelh, ngraham
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180210/b48f7fb7/attachment-0001.html>