[Merge] lp:~renatofilho/buteo-sync-plugins-contacts/new-code into lp:buteo-sync-plugins-contacts

Michael Sheldon michael.sheldon at canonical.com
Tue Jul 21 13:47:07 UTC 2015


Up to line 7000, Launchpad can't seem to handle more than 5000 lines, so I'll include further remarks in comments:


Line 5137
+        QContactExtendedDetail group;
+        group.setName(UContactsCustomDetail::FieldGroupMembershipInfo);
+        group.setData("6");

Can you add a comment explaining the significance of "6"? (Judging from later code I'm guessing this is the 'My Contacts' group?)


Line 5472
+        protocolMap.insert(QContactOnlineAccount::ProtocolJabber, "JABBER");
+        protocolMap.insert(QContactOnlineAccount::ProtocolAim, "AIM");
+        protocolMap.insert(QContactOnlineAccount::ProtocolIcq, "ICQ");
+        protocolMap.insert(QContactOnlineAccount::ProtocolMsn, "MSN");
+        protocolMap.insert(QContactOnlineAccount::ProtocolQq, "QQ");
+        protocolMap.insert(QContactOnlineAccount::ProtocolYahoo, "YAHOO");
+        protocolMap.insert(QContactOnlineAccount::ProtocolSkype, "SKYPE");
+        protocolMap.insert(QContactOnlineAccount::ProtocolIrc, "IRC");

I notice that this list doesn't include GOOGLE_TALK but an earlier one did, should they not match?


Line 6089
+    // keep downloader object live while GRemoreSource exists to avoid remove
+    // the temporary files used to store avatars

GRemoreSource -> GRemoteSource, remove -> removing


Line 6522
+                // FIXME
+                //  m_unsupportedXmlElements[accountId].insert(
+                //          c.detail<QContactGuid>().guid(),
+                //          remoteAddModContacts[i].second);
+                //  m_contactEtags[accountId].insert(c.detail<QContactGuid>().guid(), c.detail<QContactOriginMetadata>().id());
+                // c.setId(QContactId::fromString(m_contactIds[accountId].value(c.detail<QContactGuid>().guid())));
+                // m_remoteAddMods[accountId].append(c);

Can you add details about what needs fixing here? Also the same for line 6542


Line 6604
+    // TODO: If interested, check the value of error. But
+    // it is enough to say that it is a SYNC_CONNECTION_ERROR
+    //emit syncFinished (Sync::SYNC_CONNECTION_ERROR);

The code following this looks like it is now checking the type of sync error, so this TODO can probably be removed.


I also just noticed that all the updated copyright notices have a typo in the name, they should be "buteo-sync-plugins-google" not "buteo-sync-plugins-goole" (missing "g" in google).
-- 
https://code.launchpad.net/~renatofilho/buteo-sync-plugins-contacts/new-code/+merge/264335
Your team Ubuntu Phablet Team is subscribed to branch lp:buteo-sync-plugins-contacts.



More information about the Ubuntu-reviews mailing list