D24349: More fixes to compile without implicit conversion from ASCII/ByteArray

David Faure noreply at phabricator.kde.org
Tue Nov 26 20:07:45 GMT 2019


dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> ahmadsamir wrote in slavebase.h:979
> A B C, nothing changed inside make it const, D E F.

Yep ;)

Hmm, also it's in the middle of member variables, better not mix them up.
Can you make the method private, if it's only used by slavebase.cpp?
[if not, then it needs documentation and @since and a better name...]

> kfileplacesmodel.cpp:275
>  
> -    if (root.first().isNull() || !QFile::exists(file)) {
> +    const static QString fsBookmarks = QStringLiteral("KFile System Bookmarks");
>  

This is more commonly written as "static const"

> ahmadsamir wrote in main.cpp:199
> In my defence, since that looks a bit too crazy even to me, I was worried about files with names that have encoding issues, e.g. the dreaded �; but looking up just a little at the code I see that fileList was written by the code, so if the encoding did go south, that ship has sailed and already has sunk :)

In any case, encoding issues are for QString<->QByteArray conversions. Any time we can stick to QString<->QString like here, we're safe :)

> kpasswdservertest.cpp:304
>             KIO::AuthInfo info;
> -           info.url = QUrl("http://www.example.com/test" + QString::number(i) + ".html");
> +           info.url = QUrl(QLatin1String("http://www.example.com/test") + QString::number(i) + QStringLiteral(".html"));
>             authInfos << info;

[strange mix of QLatin1String and QStringLiteral, but harmless]

REPOSITORY
  R241 KIO

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

To: ahmadsamir, dfaure, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20191126/2a9cc2d6/attachment.html>


More information about the Kde-frameworks-devel mailing list