Code review comment for lp:~ken-vandine/libappindicator/gtk3

  review approve

On Tue, 2011-01-25 at 16:07 +0000, Ken VanDine wrote:
> Ken VanDine has proposed merging lp:~ken-vandine/libappindicator/gtk3 into lp:libappindicator.
>
> Requested reviews:
> Indicator Applet Developers (indicator-applet-developers)
>
> For more details, see:
> https://code.launchpad.net/~ken-vandine/libappindicator/gtk3/+merge/47411
>
> More GTK fixes, the /indicator-application/libappindicator/set_menu test is failing though and I am not sure why.
> differences between files attachment (review-diff.txt)
> === modified file 'bindings/Makefile.am'
> --- bindings/Makefile.am 2009-12-16 19:32:39 +0000
> +++ bindings/Makefile.am 2011-01-25 16:06:53 +0000
> @@ -1,3 +1,7 @@
> +if USE_GTK3
> +SUBDIRS = mono
> +else
> SUBDIRS = \
> mono \
> python
> +endif
>
> === modified file 'docs/reference/Makefile.am'
> --- docs/reference/Makefile.am 2010-12-04 02:44:18 +0000
> +++ docs/reference/Makefile.am 2011-01-25 16:06:53 +0000
> @@ -9,7 +9,12 @@
> # of using the various options.
>
> # The name of the module, e.g. 'glib'.
> +if USE_GTK3
> +DOC_MODULE=libappindicator3
> +else
> DOC_MODULE=libappindicator
> +endif
> +
>
> # Uncomment for versioned docs and specify the version of the module, e.g. '2'.
> #DOC_MODULE_VERSION=2
> @@ -91,7 +96,7 @@
> # e.g. GTKDOC_CFLAGS=-I$(top_srcdir) -I$(top_builddir) $(GTK_DEBUG_FLAGS)
> # e.g. GTKDOC_LIBS=$(top_builddir)/gtk/$(gtktargetlib)
> GTKDOC_CFLAGS=-I$(top_srcdir) -I$(top_srcdir)/src -I$(top_srcdir)/src $(LIBRARY_CFLAGS)
> -GTKDOC_LIBS=$(top_builddir)/src/libappindicator.la $(LIBRARY_LIBS)
> +GTKDOC_LIBS=$(top_builddir)/src/$(DOC_MODULE).la $(LIBRARY_LIBS)
>
> # This includes the standard gtk-doc make rules, copied by gtkdocize.
> include $(top_srcdir)/gtk-doc.local.make
>
> === modified file 'example/Makefile.am'
> --- example/Makefile.am 2010-12-04 03:37:58 +0000
> +++ example/Makefile.am 2011-01-25 16:06:53 +0000
> @@ -1,3 +1,9 @@
> +if USE_GTK3
> +VER=3
> +else
> +VER=
> +endif
> +
>
> check_PROGRAMS = \
> simple-client
> @@ -17,7 +23,7 @@
>
> simple_client_LDADD = \
> $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> EXTRA_DIST = \
> simple-client-test-icon.png
>
> === modified file 'example/simple-client.c'
> --- example/simple-client.c 2010-11-09 23:01:37 +0000
> +++ example/simple-client.c 2011-01-25 16:06:53 +0000
> @@ -86,13 +86,14 @@
> {
> GtkWidget *target = (GtkWidget *)data;
>
> - gtk_widget_set_sensitive (target, !GTK_WIDGET_IS_SENSITIVE (target));
> + gtk_widget_set_sensitive (target, !gtk_widget_is_sensitive (target));
> }
>
> static void
> image_clicked_cb (GtkWidget *widget, gpointer data)
> {
> - gtk_image_set_from_stock (GTK_IMAGE (GTK_IMAGE_MENU_ITEM (widget)->image),
> + gtk_image_set_from_stock (GTK_IMAGE (gtk_image_menu_item_get_image (
> + GTK_IMAGE_MENU_ITEM (widget))),
> GTK_STOCK_OPEN, GTK_ICON_SIZE_MENU);
> }
>
>
> === modified file 'src/Makefile.am'
> --- src/Makefile.am 2011-01-13 23:36:29 +0000
> +++ src/Makefile.am 2011-01-25 16:06:53 +0000
> @@ -45,7 +45,7 @@
>
> DISTCLEANFILES += app-indicator-enum-types.c
>
> -libappindicatorincludedir=$(includedir)/libappindicator$(VER)-0.1/libappindicator
> +libappindicatorincludedir=$(includedir)/libappindicator-0.1/libappindicator
>
> libappindicator_headers = \
> app-indicator.h
>
> === modified file 'src/appindicator3-0.1.pc.in'
> --- src/appindicator3-0.1..pc.in 2011-01-12 15:01:09 +0000
> +++ src/appindicator3-0.1.pc.in 2011-01-25 16:06:53 +0000
> @@ -4,7 +4,7 @@
> bindir=@bindir@
> includedir=@includedir@
>
> -Cflags: -I${includedir}/libappindicator3-0.1
> +Cflags: -I${includedir}/libappindicator-0.1
> Requires: dbusmenu-glib-0.4 gtk+-3.0
> Libs: -L${libdir} -lappindicator3
>
>
> === modified file 'tests/Makefile.am'
> --- tests/Makefile.am 2010-12-08 21:58:26 +0000
> +++ tests/Makefile.am 2011-01-25 16:06:53 +0000
> @@ -1,3 +1,9 @@
> +if USE_GTK3
> +VER=3
> +else
> +VER=
> +endif
> +
>
> check_PROGRAMS = \
> test-libappindicator \
> @@ -31,7 +37,7 @@
>
> test_libappindicator_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> #########################################
> ## test-libappindicator-dbus-client
> @@ -48,7 +54,7 @@
>
> test_libappindicator_dbus_client_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> #########################################
> ## test-libappindicator-dbus-server
> @@ -65,7 +71,7 @@
>
> test_libappindicator_dbus_server_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> #########################################
> ## test-libappindicator-status-client
> @@ -82,7 +88,7 @@
>
> test_libappindicator_status_client_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> #########################################
> ## test-libappindicator-status-server
> @@ -99,7 +105,7 @@
>
> test_libappindicator_status_server_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> #########################################
> ## test-libappindicator-fallback
> @@ -115,7 +121,7 @@
>
> test_libappindicator_fallback_watcher_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> test_libappindicator_fallback_item_SOURCES = \
> test-libappindicator-fallback-item.c
> @@ -127,7 +133,7 @@
>
> test_libappindicator_fallback_item_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
> test-libappindicator-fallback: test-libappindicator-fallback-watcher test-libappindicator-fallback-item Makefile.am
> @echo "#!/bin/bash" > $@
> @@ -192,5 +198,5 @@
>
> test_simple_app_LDADD = \
> $(TESTDEPS_LIBS) $(LIBRARY_LIBS) \
> - $(top_builddir)/src/libappindicator.la
> + $(top_builddir)/src/libappindicator$(VER).la
>
>