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 771839 - x-content-bar: port to use G_DECLARE* type declarations
x-content-bar: port to use G_DECLARE* type declarations
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
master
Other All
: Normal minor
: ---
Assigned To: Neha Yadav
Nautilus Maintainers
Depends on:
Blocks: 771777
 
 
Reported: 2016-09-22 15:04 UTC by Ernestas Kulik
Modified: 2016-10-09 18:23 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
x-content-bar : port to G_DECLARE* type declaration (9.14 KB, patch)
2016-10-08 21:48 UTC, Neha Yadav
none Details | Review
x-content-bar : port to G_DECLARE* type declaration (8.92 KB, patch)
2016-10-09 08:57 UTC, Neha Yadav
none Details | Review
x-content-bar : port to G_DECLARE* type declaration (8.99 KB, patch)
2016-10-09 16:06 UTC, Neha Yadav
none Details | Review
x-content-bar : port to G_DECLARE* type declaration (8.99 KB, patch)
2016-10-09 18:18 UTC, Neha Yadav
committed Details | Review

Description Ernestas Kulik 2016-09-22 15:04:43 UTC
See bug 771777.
Comment 1 Neha Yadav 2016-10-06 20:43:03 UTC
Can you please assign me this bug
Comment 2 Neha Yadav 2016-10-08 21:48:01 UTC
Created attachment 337256 [details] [review]
x-content-bar : port to G_DECLARE* type declaration

Currently we are using the old GObject class declarations, which have two
problems.

One problem is that we cannot use smart pointers like g_autoptr. The other
problem is the boilerplate code generated that makes the code less readable,
so harder to understand.

To fix this use G_DECLARE* type.
Comment 3 Ernestas Kulik 2016-10-09 08:40:48 UTC
Review of attachment 337256 [details] [review]:

Have you tried building with your changes?

::: src/nautilus-x-content-bar.c
@@ -54,3 @@
 };
 
-G_DEFINE_TYPE (NautilusXContentBar, nautilus_x_content_bar, GTK_TYPE_INFO_BAR)

Why remove this?

::: src/nautilus-x-content-bar.h
@@ +34,3 @@
 							    const char * const*  x_content_types);
 
+G_DECLARE_FINAL_TYPE (NautilusXContentBar, nautilus_x_content_bar, NAUTILUS, X_CONTENT_BAR, GtkInfoBar)

Keep this above function declarations, for consistency.
Check other source files for reference.
Comment 4 Neha Yadav 2016-10-09 08:57:30 UTC
Created attachment 337261 [details] [review]
x-content-bar : port to G_DECLARE* type declaration

Currently we are using the old GObject class declarations, which have two
problems.

One problem is that we cannot use smart pointers like g_autoptr. The other
problem is the boilerplate code generated that makes the code less readable,
so harder to understand.

To fix this use G_DECLARE* type.
Comment 5 Ernestas Kulik 2016-10-09 10:50:35 UTC
Review of attachment 337261 [details] [review]:

Splendid! Only one nitpick. :)

::: src/nautilus-x-content-bar.h
@@ +35,1 @@
+GtkWidget	*nautilus_x_content_bar_new (GMount               *mount,

Now that you have removed the tabs between the function name and the opening parenthesis, could you remove all tabs here and realign the parameters using spaces?
Comment 6 Neha Yadav 2016-10-09 16:06:59 UTC
Created attachment 337281 [details] [review]
x-content-bar : port to G_DECLARE* type declaration

Currently we are using the old GObject class declarations, which have two
problems.

One problem is that we cannot use smart pointers like g_autoptr. The other
problem is the boilerplate code generated that makes the code less readable,
so harder to understand.

To fix this use G_DECLARE* type.
Comment 7 Ernestas Kulik 2016-10-09 16:46:49 UTC
Review of attachment 337281 [details] [review]:

Really, really close!

::: src/nautilus-x-content-bar.h
@@ +35,2 @@
+GtkWidget	*nautilus_x_content_bar_new (GMount               *mount,
+                                const char * const*  x_content_types);

What I meant by my remark was “use the ‘approved’ code style”, that is, leave a single space between the type and the name (the pointer asterisk is part of the name in this case) and align parameter types and names by their first letters. Again, no tabs anywhere (there is still one).

I would suggest cross-referencing this against other headers, but the style differs for when there are clusters of declarations and when there is only a single one.

I hope I am making it clear to you, but definitely ask on IRC if you have doubts. This might seem trivial, but it is more troublesome to maintain code, which uses differing styles, than having to touch up your patch a few times. We definitely appreciate your work and it hurts me to keep pestering you about this, so please try to not lose motivation over something trivial. :)
Comment 8 Neha Yadav 2016-10-09 17:54:29 UTC
Sorry I troubled you a lot and thank you for noticing my small mistake.
What I understood is that * is a part of name so, according to that this is the output.

GtkWidget *nautilus_x_content_bar_new (GMount               *mount,
                                       const char * const *x_content_types);

Let me know if I am wrong
Comment 9 Ernestas Kulik 2016-10-09 18:11:10 UTC
(In reply to Neha Yadav from comment #8)
> Sorry I troubled you a lot and thank you for noticing my small mistake.
> What I understood is that * is a part of name so, according to that this is
> the output.
> 
> GtkWidget *nautilus_x_content_bar_new (GMount               *mount,
>                                        const char * const *x_content_types);
> 
> Let me know if I am wrong

You are almost correct.

GtkWidget *nautilus_x_content_bar_new (GMount             *mount,
                                       const char * const *x_content_types);

Notice the parameter names and the alignment.
Comment 10 Neha Yadav 2016-10-09 18:18:29 UTC
Created attachment 337284 [details] [review]
x-content-bar : port to G_DECLARE* type declaration

Currently we are using the old GObject class declarations, which have two
problems.

One problem is that we cannot use smart pointers like g_autoptr. The other
problem is the boilerplate code generated that makes the code less readable,
so harder to understand.

To fix this use G_DECLARE* type.
Comment 11 Ernestas Kulik 2016-10-09 18:22:24 UTC
Review of attachment 337284 [details] [review]:

Yes! Thanks. :)
Comment 12 Ernestas Kulik 2016-10-09 18:23:22 UTC
Attachment 337284 [details] pushed as 1be60ac - x-content-bar : port to G_DECLARE* type declaration