[Bug 34985] rtcp-fb and rtp-hdrext support in spec and gabble

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Mar 16 12:02:16 CET 2011


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

--- Comment #7 from Will Thompson <will.thompson at collabora.co.uk> 2011-03-16 04:02:15 PDT ---
(In reply to comment #5)
> I fixed most of your comments and updated the branch .. except those:
> 
> (In reply to comment #3)
> > I've been trying to avoid nitpicking indentation (and if (foo) vs if (foo !=
> > NULL) and other style trivia)
> 
> For the (foo) vs (foo != NULL), I though the TP convention was to only use if
> (foo) is foo was a boolean and I tried to follow that everywhere

I thought I saw some places you didn't … I could have been wrong.

For instance, in these new patches:

-  return gabble_presence_resource_has_caps (presence,
+  return presence && gabble_presence_resource_has_caps (presence,
       gabble_jingle_session_get_peer_resource (content->session),
       gabble_capability_set_predicate_has, cap);

:)

-  type = lm_message_node_get_attribute (node, "type");
+ type = lm_message_node_get_attribute (node, "type");
   if (type == NULL)
     return NULL;

indentation…

> > +static gboolean
> > +jingle_feedback_message_compare (const JingleFeedbackMessage *fb1,
> > +    const JingleFeedbackMessage *fb2)
> > +{
> > +  if (!g_ascii_strcasecmp (fb1->type, fb2->type) &&
> > +      !g_ascii_strcasecmp (fb1->subtype, fb2->subtype))
> > +    return 0;
> > +  else
> > +    return 1;
> > +}
> > 
> > FALSE and TRUE please.
> 
> It's not a gboolean, its 0, 1 or -1 for equal, larger or smaller..

The function's return type is gboolean …

> I guess I could do a = g_ascii_strcasecmp (fb1->type, fb2->type); b =
> g_ascii_strcasecmp (fb1->subtype, fb2->subtype)); return a ? a : b; if I cared
> about the ordering
> 
> > I'm balking at jingle_media_description_simplify() — I will come back to it. It
> > seems excessively complicated.
> 
> Added a bunch of comments to make it clearer what it's supposed to do.

Yeah, it's clearer now; cheers.

-- 
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