D10414: Add move semantics support to KIO::UDSEntry.
David Faure
noreply at phabricator.kde.org
Sun Feb 11 20:16:32 UTC 2018
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
I actually see little point in verifying that *compiler generated* code copies all members correctly. We can trust the compiler :-)
So, to me this looks ok (minus the last two small comments).
If however you were to implement a more complete checking of fields, I would recommend against hashing (how do you debug it when the expected value is wrong?), and to go instead for a string representation (concatenate data using a separator [doesn't matter if the separator is used in one of the values, there's no splitting back needed], QCOMPARE the two strings, then failures *can* be debugged). But as I said, this doesn't seem necessary here.
INLINE COMMENTS
> udsentrytest.cpp:227
> +/**
> + * Test to verify that move semantics work. This is only useful when ran through a profiling or dubugging tool.
> + */
typo: it's debugging, with an 'e'
> udsentrytest.cpp:233
> + QTemporaryFile file;
> + QCOMPARE(file.open(), true);
> + const QByteArray filePath = QFile::encodeName(file.fileName());
That one, however, could be QVERIFY(), that's what it's for ;)
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D10414
To: markg, dfaure
Cc: apol, #frameworks, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180211/348e2af3/attachment.html>
More information about the Kde-frameworks-devel
mailing list