[Bug 24120] Refactor the McdDispatcher

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Tue Sep 29 16:32:06 CEST 2009


http://bugs.freedesktop.org/show_bug.cgi?id=24120





--- Comment #2 from Sjoerd Simons <sjoerd at luon.net>  2009-09-29 07:32:05 PST ---
Currently up to 26cf02b0767fc85d16827482252b89978c53f7d3 with my review (most
things are nitpicks, generally stuff looks good):

* operation_list_active seems a bit of a premature optimisation, existing code
  though

* if (approved_handler != NULL && approved_handler[0] != '\0')
    reminds me that we have EMP_STR_EMPTY in empathy, which is a macro that
    does exactly this, might be useful to put in tp-glib as well as it's a
    commen telepathy pattern.

* mcd_dispatcher_get_possible_handlers -> _get_ functions returning refs is
  unexpected (again existing code though)

* 5171899833d482548f3ff96143e35ee4d0619ead
  +     * dup'd bus name (string) => arbitrary non-NULL pointer */
    arbitrary? really ? from the looks it seems it means dummy pointer

  function names seems a bit unobvious:
    _mcd_dispatch_operation_set_handler_failed ->
      _mcd_dispatch_operation_mark_handler_failed or
      _mcd_dispatch_operation_add_failed_handler

   _mcd_dispatch_operation_get_handler_failed ->
     _mcd_dispatch_operation_handler_failed or
     _mcd_dispatch_operation_handler_has_failed

* activateable property on McdClientProxy should probably be readable as well


-- 
Configure bugmail: http://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