[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