← Back to team overview

ayatana-commits team mailing list archive

Re: [Merge] lp:~ted/dbusmenu/kill-dbus-glib into lp:dbusmenu

 

Review: Approve static
I don't know anything about gdbus so this is mostly a static generic review.

- You introduced an xslt file to filter out dox elements from the interface xml files, I assume this is because gdbus has a problem with them. Do they suggest other ways to document interfaces?

- There seems to be a vast amount of duplicated code for error handling. Can't this be factorized? I am thinking about blocks like these:

"""
if (priv->root == NULL) {
  g_dbus_method_invocation_return_error(invocation,
  error_quark(),
  NO_VALID_LAYOUT,
  "There currently isn't a layout in this server");
  return;
}
"""

"""
DbusmenuMenuitem * mi = dbusmenu_menuitem_find_id(priv->root, id);

if (mi == NULL) {
  g_dbus_method_invocation_return_error(invocation,
    error_quark(),
    INVALID_MENUITEM_ID,
    "The ID supplied %d does not refer to a menu item we have",
    parent);
    return;
}
"""

I think having macros like return_if_no_layout(priv) and get_menuitem_or_return(priv, id) would make the code shorter and easier to read.
-- 
https://code.launchpad.net/~ted/dbusmenu/kill-dbus-glib/+merge/42543
Your team ayatana-commits is subscribed to branch lp:dbusmenu.



References