Thanks for the review, Niall. I will update the patch. Regards,
Jedy On Thu, 2008-03-13 at 19:06 -0700, Niall Power wrote: > Hi Jedy. > > First of all, let me say thanks for an excellent job on this. > > I have a few minor comments on the code. > > map.c: > ----- > draw_point (): The MapPrivate *priv variable is unused. > map_draw_timezones (): The "area" parameter is unused in this function > do_redraw (): The "area" parameter is unused in this function. > > timezone.c: > --------- > on_all_timezones_added(): The prototype and the actual function spec are > inconsistent: > > 80 static void on_all_timezones_added(GtkWidget *widget, gpointer > user_data1, > 81 gpointer user_data); > > 133 static void > 134 on_all_timezones_added(GtkWidget *widget, gpointer user_data1, > 135 gpointer user_data2) > > Also, in on_alltimezones_added(), user_data1 is never used, and looking at > the g_signal_emit() call that generates this signal in map.c, it appears that > the second paramater (user_data1) is not user data but instead a > continent_item. > > Nits: Line 533 and 540: Indentation is inconsistent. > > I think there is some visual adjustment needed also, but I think this can be > easily corrected later and I don't it should block the approval of this code > review and getting the code integrated into the workspace. > There should be more vertical padding between the map and the timezone combo > boxes. I think it should be 6 or 12 pixels. > > Great job! > > Thanks, > Niall. > -- > This message posted from opensolaris.org > _______________________________________________ > caiman-discuss mailing list > caiman-discuss at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/caiman-discuss -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/caiman-discuss/attachments/20080317/43fba9d5/attachment.html>
