[Bug 20034] Avatar cache reference implementation
bugzilla-daemon at freedesktop.org
bugzilla-daemon at freedesktop.org
Tue Sep 7 21:09:35 CEST 2010
https://bugs.freedesktop.org/show_bug.cgi?id=20034
--- Comment #8 from Andre Moreira Magalhaes <andrunko at gmail.com> 2010-09-07 12:09:35 PDT ---
Coding style issues:
+ Q_FOREVER {
+ tmpDir = QDir::tempPath() +
QString::fromLatin1("/avatars-%1").arg(i++);
+ if (QDir().mkdir(tmpDir))
+ break;
+ }
I am afraid that this code will run forever if we don't have write access to
QDir::tempPath(). Not that this will ever going to happen. Not a blocker
anyway.
+ setenv ("XDG_CACHE_HOME", a.constData(), true);
+ QVERIFY (SmartDir(tmpDir).removeDirectory());
Extra spaces here. Also there are many other places where there are extra
spaces in TestContactsAvatar::createContactWithFakeAvatar.
All coding style in HACKING should be applied to glib/C code inside written a
C++ source. Especially lowerCamelCase for variable names and no extra space
before (. For example:
+ TpHandleRepoIface *service_repo = tp_base_connection_get_handles (
+ (TpBaseConnection *) mConnService, TP_HANDLE_TYPE_CONTACT);
+ handle = tp_handle_ensure (service_repo, id, NULL, NULL);
Should be:
+ TpHandleRepoIface *serviceRepo = tp_base_connection_get_handles(
+ (TpBaseConnection *) mConnService, TP_HANDLE_TYPE_CONTACT);
+ handle = tp_handle_ensure(serviceRepo, id, NULL, NULL);
+ * AvatarRetrived should NOT be called */
typo in AvatarRetrieved
+ SmartDir(const QString &path):QDir(path) { }
Should be:
+ SmartDir(const QString &path) : QDir(path) { }
+ Q_FOREACH(QFileInfo info, list) {
should be:
+ Q_FOREACH (QFileInfo info, list) {
+ }
+ else {
should be:
+ } else {
+ QString path = cacheDir + QString::fromLatin1("/telepathy/avatars/") +
connection->cmName() + QString::fromLatin1("/") + connection->protocolName();
Avoid using QString + operator. This code should be changed to something like:
QString path = QString(QLatin1String("%1/telepathy/avatars/%2/%3")).
arg(cacheDir).arg(connection->cmName()).arg(connection->protocolName());
Do this for all places where you are using the + operator and
QString::fromLatin1().
Also try to declare variables right before the usage if possible. utils.cpp
could have some c++ love there. But don't bother changing this, not a blocker
at all.
Sorry being so pedantic about coding style :/.
--
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
More information about the telepathy-bugs
mailing list