Created attachment 135158 [details] [review]
Use GtkBuilder instead of libglade

There are changes in the files m4/intltool.m4 and po/Makefile.in.in but I think that don't matter because these files are generated when you execute the autogen

If not, please tell me

gnome-system-tools is basically unmaintained so I wonder who could review this.

Comment on attachment 135158 [details] [review]
Use GtkBuilder instead of libglade

Please drop all the changes in m4/intltool.m4 and po/Makefile.in.in from the patch.
If those files are modified at build time and are under version control, that's another bug.

>diff --git a/src/boot/boot-append-gui.c b/src/boot/boot-append-gui.c

>+           if (!gtk_builder_add_from_file (builder, tool->ui_path, &error))
>+             {
>+               g_warning ("Couldn't load builder file: %s", error->message);
>+               g_error_free (error);
>+             }

You should probably return NULL here.

>@@ -495,7 +502,7 @@ boot_append_gui_destroy (BootAppendGui *gui)

>+			 g_object_unref (G_OBJECT (gui->builder));

No need to cast to GObject* here.

>diff --git a/src/boot/boot-settings.c b/src/boot/boot-settings.c

>+	if (!gtk_builder_add_from_file (builder, ui_path, &error))
>+	{
>+		g_warning ("Couldn't load builder file: %s", error->message);
>+		g_error_free (error);
>+	}

You should return NULL here.

>@@ -441,7 +448,7 @@ boot_settings_gui_destroy (BootSettingsGui *gui)

>+		g_object_unref (G_OBJECT (gui->builder));

Useless cast.

>diff --git a/src/boot/callbacks.c b/src/boot/callbacks.c
>@@ -91,20 +91,27 @@ static BootImageEditor *
>+	if (!gtk_builder_add_from_file (builder, boot_dialog, &error))
>+	{
>+		g_warning ("Couldn't load builder file: %s", error->message);
>+		g_error_free (error);
>+	}

>@@ -189,20 +196,26 @@ static BootAppendEditor *

>+	if (!gtk_builder_add_from_file (builder, boot_dialog, &error))
>+	{
>+		g_warning ("Couldn't load builder file: s", error->message);
>+		g_error_free (error);
>+	}
>+	new->dialog = GTK_DIALOG (gtk_builder_get_object (builder, "boot_dialog"));

Same here.

>+	g_signal_connect (G_OBJECT (gtk_builder_get_object (builder, "boot_dialog_help")),
> 			  "clicked", G_CALLBACK (on_boot_help_button_clicked), NULL);

Useless cast.


Apart from these nitpicks you should also:
- add the converted UI file to the patch (using git-add) and remove the glade one (with git-rm)
- replace the glade file in the po/POTFILES.in list with the UI one, prefixing it with [type: gettext/glade]
- remove the glade bits from configure.ac (and Makefile.am if appropriate)

Javier: Willing to come up with an updated patch that covers Cosimo's comments?

Created attachment 138643 [details] [review]
Use GtkBuilder instead of libglade v2

Sorry, I completely forgot this patch. Thank you André for the reminder ;)

Should we just commit this?

Let's ask Milan if he's fine with getting this in.

I've had a look at the patch and it looks good to me, I trust you for the details. Tested here and that works, only updated to take into account changes made to interfaces/users.ui since the patch was posted.

Much thanks for the patch Javier (and Cosimo for the review), it's really nice to get such great work in the g-s-t! And that will make maintaining easier now that the .glade files are not duplicated - not speaking about the g-s-t eventually moving towards GNOME 3.0. Now that you've worked with the g-s-t codebase, what about helping us further? ;-)

Pushed as 0ac0ac85f50b6bd016249278e946ad3e0862fc5a and d69ce5c2c9df3b860adddb41879b40585b67e264. I've updated the Wiki page.