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 786815 - Port NautilusFloatingBar to G_DECLARE_*_TYPE & G_DEFINE_TYPE_*
Port NautilusFloatingBar to G_DECLARE_*_TYPE & G_DEFINE_TYPE_*
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks: 771777
 
 
Reported: 2017-08-25 19:47 UTC by Vyas Giridhar
Modified: 2017-08-30 08:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
floating-bar: port to GLib type declaration macros. (15.19 KB, patch)
2017-08-25 19:47 UTC, Vyas Giridhar
needs-work Details | Review
floating-bar: port to GLib type declaration macros. (11.14 KB, patch)
2017-08-28 11:43 UTC, Vyas Giridhar
none Details | Review
floating-bar: port to GLib type declaration macros. (11.31 KB, patch)
2017-08-29 16:54 UTC, Vyas Giridhar
none Details | Review
floating-bar: port to GLib type declaration macros. (11.31 KB, patch)
2017-08-29 17:00 UTC, Vyas Giridhar
committed Details | Review

Description Vyas Giridhar 2017-08-25 19:47:02 UTC
.
Comment 1 Vyas Giridhar 2017-08-25 19:47:53 UTC
Created attachment 358442 [details] [review]
floating-bar: port to GLib type declaration macros.

This patch ports the declaration of NautilusFloatingBar
to G_DECLARE_FINAL_TYPE and G_DEFINE_TYPE_WITH_PRIVATE
Comment 2 Ernestas Kulik 2017-08-28 07:05:10 UTC
Review of attachment 358442 [details] [review]:

You shouldn’t be explaining in the commit message what changes you made in the code, as it’s obvious from the code itself. Rather, provide motivations, etc.

Otherwise the patch looks mostly good.

::: src/nautilus-floating-bar.c
@@ +58,3 @@
 static guint signals[NUM_SIGNALS] = { 0, };
 
+G_DEFINE_TYPE_WITH_PRIVATE (NautilusFloatingBar, nautilus_floating_bar,

Private data does not make sense here. Move the type struct here and use that to hold data.

@@ -76,3 +76,3 @@
 nautilus_floating_bar_finalize (GObject *obj)
 {
-    NautilusFloatingBar *self = NAUTILUS_FLOATING_BAR (obj);
+    NautilusFloatingBar *self;

Unrelated change.

@@ +84,2 @@
     nautilus_floating_bar_remove_hover_timeout (self);
+

Unrelated change.

@@ -91,3 +96,3 @@
                                     GParamSpec *pspec)
 {
-    NautilusFloatingBar *self = NAUTILUS_FLOATING_BAR (object);
+    NautilusFloatingBar *self;

Unrelated change.

@@ +138,3 @@
+    NautilusFloatingBar *self;
+
+    self = NAUTILUS_FLOATING_BAR (object);

Don’t make unrelated changes.

@@ +195,3 @@
+    NautilusFloatingBarPrivate *priv;
+
+	priv = nautilus_floating_bar_get_instance_private (self);

Tab indentation.

@@ +197,3 @@
+	priv = nautilus_floating_bar_get_instance_private (self);
+
+	if (priv->hover_timeout_id != 0)

Tab indentation.

@@ +224,3 @@
     CheckPointerData *data = user_data;
     gint pointer_y = -1;
+	NautilusFloatingBarPrivate *priv;

You indented with a tab here.

@@ +287,2 @@
                                                        check_pointer_timeout, data,
                                                        check_pointer_data_free);

Arguments are misaligned here.

::: src/nautilus-floating-bar.h
@@ +30,2 @@
 #define NAUTILUS_TYPE_FLOATING_BAR nautilus_floating_bar_get_type()
+G_DECLARE_FINAL_TYPE (NautilusFloatingBar, nautilus_floating_bar, NAUTILUS, FLOATING_BAR, GtkBox);

Semicolon here is unnecessary.

@@ +33,1 @@
 struct _NautilusFloatingBar {

No reason to leave this in the header.
Comment 3 Vyas Giridhar 2017-08-28 11:43:00 UTC
Created attachment 358584 [details] [review]
floating-bar: port to GLib type declaration macros.
Comment 4 Ernestas Kulik 2017-08-29 08:28:57 UTC
Review of attachment 358584 [details] [review]:

Looks mostly good, but now your commit message has no body.

::: src/nautilus-floating-bar.c
@@ +28,3 @@
 #define HOVER_HIDE_TIMEOUT_INTERVAL 100
 
+struct _NautilusFloatingBar {

Opening brace on a new line.
Comment 5 Vyas Giridhar 2017-08-29 16:54:22 UTC
Created attachment 358708 [details] [review]
floating-bar: port to GLib type declaration macros.

Porting Nautilus object declarations to G_DECLARE_*_TYPE will
reduce a a huge number of boilerplate code.

That will make nautilus code easier for newcomers and others alike.
Comment 6 Vyas Giridhar 2017-08-29 17:00:50 UTC
Created attachment 358710 [details] [review]
floating-bar: port to GLib type declaration macros.

Porting Nautilus object declarations to G_DECLARE_*_TYPE will
reduce a huge number of boilerplate code.

That will make nautilus code easier for newcomers and others alike.
Comment 7 Ernestas Kulik 2017-08-30 08:24:17 UTC
Review of attachment 358710 [details] [review]:

Yup.
Comment 8 Ernestas Kulik 2017-08-30 08:26:30 UTC
Attachment 358710 [details] pushed as cad0a3e - floating-bar: port to GLib type declaration macros.