http://bugzilla.moblin.org/show_bug.cgi?id=5188
--- Comment #16 from pohly <[email protected]> 2009-11-13 02:57:31 PST --- (In reply to comment #15) > Is our own DevInf perhaps becoming so large that it doesn't fit into the > outgoing message? Wrong issue number... (In reply to comment #14) > I just pushed the updated code to obex branch. > Patrick, could you give a review? Dbus Server: SAN parsing Fix a bug in san parsing. Can you please elaborate in your commit message what the problem was? You seem to be fixing two unrelated problems in one commit. Better split the commit (rebase -i, "edit" this commit, then git reset, git add -i, git commit, ...). Same with the combined "flush mapping immediately + refresh-from-remote" patch. Regarding the immediate flushing: this is kind of inefficient when adding single entries to the mapping. As a short-term solution it is okay, but I'd like to have an issue opened that reminds us of improving this. It was mentioned in an email that libsyncml didn't set the message type of messages sent to us correctly and that a workaround in obexd+SyncEvolution was necessary to handle this (something like accepting message without type and then letting SyncEvolution guess the type based on the content). I don't see that in the patch series. Is this workaround no longer necessary? Server Alerted Sync: SAN generation In the patch which adds new properties, can you update Cmdline.cpp unit tests accordingly? I've fixed the tests so that they pass in "master"; see my commits to get an idea what you have to update. Indention in that patch is inconsistent: + /* Create the transport agent */ + try { + m_agent = createTransportAgent(); + m_agent->setContentType (TransportAgent::m_contentTypeServerAlertedNotificationDS); + int retry = 0; + while (retry++ < retries) + { It should be try { m_agent... Please update the description of "syncURL" so that it documents the format of the OBEX pseudo-url. "syncURL" is mentioned in the comment for the SAN generation patch, but is the sync URL used at all in that patch? The actual new use of "syncURL" is in the next patch: Obex Client Transport: in-process obex client (binding over bluetooth, #5188) There it checks for "bt://" as prefix. I'd prefer obex-bt:// because in theory, there might be other transports over Bluetooth. The constructor of ObexTransport is also connecting. I think it would be better to put this into a separate ObexTransport::connect() method which is called directly after creating the instance in SyncContext::createTransportAgent(). -- Configure bugmail: http://bugzilla.moblin.org/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are watching someone on the CC list of the bug. _______________________________________________ Syncevolution-issues mailing list [email protected] http://lists.syncevolution.org/listinfo/syncevolution-issues
