After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 735073 - [PATCHes] Use GtkBuilder.
[PATCHes] Use GtkBuilder.
Status: RESOLVED FIXED
Product: four-in-a-row
Classification: Applications
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: four-in-a-row-maint
four-in-a-row-maint
Depends on:
Blocks:
 
 
Reported: 2014-08-19 20:47 UTC by Arnaud B.
Modified: 2014-09-15 23:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Remove tabs. (9.62 KB, patch)
2014-08-19 20:47 UTC, Arnaud B.
accepted-commit_now Details | Review
Use GtkBuilder. (12.87 KB, patch)
2014-08-19 20:48 UTC, Arnaud B.
reviewed Details | Review
Use GtkBuilder. (12.78 KB, patch)
2014-08-20 00:12 UTC, Arnaud B.
accepted-commit_after_freeze Details | Review
Remove tabs. (1.12 KB, patch)
2014-08-20 00:12 UTC, Arnaud B.
rejected Details | Review
Use GtkBuilder. (12.74 KB, patch)
2014-08-29 21:45 UTC, Arnaud B.
committed Details | Review
Remove tabs. (15.43 KB, patch)
2014-09-15 22:56 UTC, Arnaud B.
committed Details | Review

Description Arnaud B. 2014-08-19 20:47:56 UTC
Created attachment 283923 [details] [review]
Remove tabs.

Now that the UI is frozen, let’s change the code! =D (Didn’t I already told that somewhere?)

The first patch is only cleaning, the second does the job.
Comment 1 Arnaud B. 2014-08-19 20:48:21 UTC
Created attachment 283924 [details] [review]
Use GtkBuilder.
Comment 2 Michael Catanzaro 2014-08-19 22:53:51 UTC
Review of attachment 283923 [details] [review]:

OK
Comment 3 Michael Catanzaro 2014-08-19 22:57:41 UTC
Review of attachment 283924 [details] [review]:

Looks good.

::: data/Makefile.am
@@ +42,3 @@
              $(sounds_DATA) \
              $(velena_DATA) \
+	         $(appdata_in_files) \

Make sure to use the same whitespace as the rest of this list.

::: src/main.c
@@ +1177,3 @@
+  gtk_style_context_add_provider_for_screen (gdk_screen_get_default (), css_provider, GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);
+
+  builder = gtk_builder_new_from_file (DATA_DIRECTORY "/four-in-a-row.ui");     // TODO errors management?

Nope, that function will abort the program if it fails, since there's nothing else to do if you have no UI.

@@ +1181,3 @@
+  window = gtk_builder_get_object (builder, "fiar-window");
+  gtk_window_set_application (GTK_WINDOW (window), application);
+  //gtk_window_set_title (GTK_WINDOW (window), _(APPNAME_LONG));

Make sure to remove this comment
Comment 4 Arnaud B. 2014-08-20 00:12:01 UTC
Created attachment 283940 [details] [review]
Use GtkBuilder.
Comment 5 Arnaud B. 2014-08-20 00:12:33 UTC
Created attachment 283941 [details] [review]
Remove tabs.

No, that’s not the same as the first. ^^
Comment 6 Michael Catanzaro 2014-08-20 15:30:45 UTC
Review of attachment 283941 [details] [review]:

OK if the file was primarily non-tabs to begin with.  (Tabs are nicer in Makefile.am.)
Comment 7 Michael Catanzaro 2014-08-20 15:35:20 UTC
Review of attachment 283941 [details] [review]:

::: data/Makefile.am
@@ +53,3 @@
 check-local: $(appdata_DATA) $(desktop_DATA)
+    $(APPDATA_VALIDATE) $(appdata_DATA)
+    $(DESKTOP_FILE_VALIDATE) $(desktop_DATA)

Actually, you have to use tabs here :)
Comment 8 Michael Catanzaro 2014-08-20 15:36:14 UTC
Review of attachment 283940 [details] [review]:

Thanks!
Comment 9 Arnaud B. 2014-08-29 21:45:14 UTC
Created attachment 284861 [details] [review]
Use GtkBuilder.

> Actually, you have to use tabs here :)
So, I won’t touch this makefile. ^^

Here is an update of the GtkBuilder patch, build on top of “Ensure buttons have the same width”[1], and of course updating the properties.

[1] https://bugzilla.gnome.org/show_bug.cgi?id=735698
Comment 10 Michael Catanzaro 2014-08-30 15:49:51 UTC
Review of attachment 284861 [details] [review]:

Looks good.
Comment 11 Michael Catanzaro 2014-09-15 21:51:21 UTC
(In reply to comment #0)
> Created an attachment (id=283923) [details] [review]
> Remove tabs.

Unfortunately this one needs rebased :(
Comment 12 Arnaud B. 2014-09-15 22:56:56 UTC
Created attachment 286242 [details] [review]
Remove tabs.

Yeah, one function had some little changes. Here is an updated “Remove tabs.” patch with new cleanings, you can apply after.
Comment 13 Michael Catanzaro 2014-09-15 23:23:51 UTC
Thanks!

Attachment 284861 [details] pushed as 992dc39 - Use GtkBuilder.
Attachment 286242 [details] pushed as f6487e3 - Remove tabs.