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
