ayatana-commits team mailing list archive
-
ayatana-commits team
-
Mailing list archive
-
Message #02639
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