Ted, here are my comments. Nothing major, mostly style comments.
/t
indicator-tracker.h
-------------------
* copyright -> 2012
* why do use G_BEGIN_DECLS? Will this file ever be compiled as C++?
* In general, I'm used to the style where when declaring a pointer variable, parameter, or struct member,
the "*" prefixes the name without any whitespace. I'm also used to the same usage for return values,
but it seems pretty common in Gtk/Glib code to use whitespace in this case...
* Do we have in internal style-guide within PS or Systems? Just wondering about things like
line length ( you have lots of very long lines > 80 chars ), or indentation rules ( tabs vs.
spaces; you use tabs for instance, whereas recommended Gtk guidelines recommend two spaces ).
GTK+ style guide dictates two spaces for indentation, and line length <= 80
( pretty standard C style ).
* Same comment as header file re: your usage of whitespace and "*"s for variables, struct members,
and param names. Your usage is inconsistent across the file.
* Indentation is messed up: indicator_tracker_class_init().
* Do you really need to add decls for all of your "static functions"?
* Any feeling on using gtk-doc style comments for properties, signals, and
public methods?
* why the "return" statements at the end of void methods?
* indicator_tracker_init() -- why do you bother initializing the priv members
app_indicators, app_proxy_cancel, and app_indicators to NULL, when you initialize
them to real objects a few lines later?
* again, a style comment... you declare indicator_cnt in the middle of the block.
Are C99 style declarations OK?
* it took me awhile to discover that using ":" in struct initialization was a GNU
C extension.
* system_watch_vanished() -- not a big fan of your decrementing 'i' at the end
of the loop, just to have it incremented in the for loop again. Isn't there
a better way you can code this ( ie. with maybe a more complex iteration
for clause then a simple i++ )?
* l528: could you explain the comment "let's go nuclear"? I *think* what your
saying is that forcing a name change causes the entire list to be retrieved?
* l535: why do you initialize the ptrs but not the guint?
* l593: why do g_warning() like in the following method?
Ted, here are my comments. Nothing major, mostly style comments.
/t
indicator-tracker.h
-------------------
* copyright -> 2012
* why do use G_BEGIN_DECLS? Will this file ever be compiled as C++?
* In general, I'm used to the style where when declaring a pointer variable, parameter, or struct member,
the "*" prefixes the name without any whitespace. I'm also used to the same usage for return values,
but it seems pretty common in Gtk/Glib code to use whitespace in this case...
eg.
struct _IndicatorTrack erIndicator { wellknown;
gchar * name;
gchar * dbus_name;
gchar * dbus_name_
gchar * dbus_object;
gchar * prefix;
gchar * icon;
};
OR
GList * indicator_ tracker_ get_indicators (IndicatorTracker * tracker);
---
indicator-tracker.c
-------------------
* copyright -> 2012
* Do we have in internal style-guide within PS or Systems? Just wondering about things like
line length ( you have lots of very long lines > 80 chars ), or indentation rules ( tabs vs.
spaces; you use tabs for instance, whereas recommended Gtk guidelines recommend two spaces ).
GTK+ style guide dictates two spaces for indentation, and line length <= 80
( pretty standard C style ).
* Same comment as header file re: your usage of whitespace and "*"s for variables, struct members,
and param names. Your usage is inconsistent across the file.
* Indentation is messed up: indicator_ tracker_ class_init( ).
* Do you really need to add decls for all of your "static functions"?
* Any feeling on using gtk-doc style comments for properties, signals, and
public methods?
* why the "return" statements at the end of void methods?
* indicator_ tracker_ init() -- why do you bother initializing the priv members
app_indicators, app_proxy_cancel, and app_indicators to NULL, when you initialize
them to real objects a few lines later?
* again, a style comment... you declare indicator_cnt in the middle of the block.
Are C99 style declarations OK?
* it took me awhile to discover that using ":" in struct initialization was a GNU
C extension.
* system_ watch_vanished( ) -- not a big fan of your decrementing 'i' at the end
of the loop, just to have it incremented in the for loop again. Isn't there
a better way you can code this ( ie. with maybe a more complex iteration
for clause then a simple i++ )?
* l528: could you explain the comment "let's go nuclear"? I *think* what your
saying is that forcing a name change causes the entire list to be retrieved?
* l535: why do you initialize the ptrs but not the guint?
* l593: why do g_warning() like in the following method?