[Bug 32611] Implement Room interface

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Jan 12 20:11:50 CET 2011


https://bugs.freedesktop.org/show_bug.cgi?id=32611

Will Thompson <will.thompson at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Status Whiteboard|                            |review-

--- Comment #10 from Will Thompson <will.thompson at collabora.co.uk> 2011-01-12 11:11:50 PST ---
+          g_set_error (error, TP_ERRORS, TP_ERROR_INVALID_ARGUMENT,
+              "TargetID and RoomID conflict.");

I think it'd be kinder to give more information. How about: "TargetID's node
part (%s) doesn't match RoomID (%s)". Ditto the one below.

(In reply to comment #8)
> > > +      g_assert (gabble_decode_jid (target_id, &a, NULL, NULL));
> > 
> > Assertion with side effects detected! Deserves a similar comment, perhaps "/*
> > JIDs that are handles must already be valid */".
> 
> Done.

No it hasn't been done for the conflict-checking section. You added comments
above the assertions, but didn't make them side-effect-free. Surely it must be
possible to avoid repeatedly decoding the JID and asserting about it?

Do we really have to make handle_text_channel_request() over three hundred
lines long? I appreciate that it was quite long before you started, so maybe
this is a job for a cleanup branch later.

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list