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 771777 - port to G_DECLARE*_TYPE
port to G_DECLARE*_TYPE
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
master
Other Linux
: Normal enhancement
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on: 771837 771838 771839 771840 771842 771844 771845 771927 771928 771929 773831 777604 777605 777606 777607 777609 777610 777700 778137 778138 778139 786712 786815 786866
Blocks:
 
 
Reported: 2016-09-21 18:01 UTC by Mohammed Sadiq
Modified: 2017-11-05 21:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
list-view: ported declaration to G_DECLARE* (1.81 KB, patch)
2017-08-21 14:57 UTC, Vyas Giridhar
none Details | Review
icon-info: ported declaration to G_DECLARE* (1.70 KB, patch)
2017-08-21 14:59 UTC, Vyas Giridhar
needs-work Details | Review
icon-info: ported declaration to G_DECLARE* (1.70 KB, patch)
2017-08-21 15:20 UTC, Vyas Giridhar
none Details | Review
search-engine-tracker: ported declaration to G_DECLARE* (2.08 KB, patch)
2017-08-21 15:27 UTC, Vyas Giridhar
needs-work Details | Review
search-engine-model: ported declaration to G_DECLARE* (2.01 KB, patch)
2017-08-21 15:30 UTC, Vyas Giridhar
none Details | Review
special-location-bar: ported declaration to G_DECLARE* (2.21 KB, patch)
2017-08-21 15:36 UTC, Vyas Giridhar
none Details | Review
list-view: ported declaration to G_DECLARE* (1.81 KB, patch)
2017-08-22 14:47 UTC, Vyas Giridhar
none Details | Review
list-view: ported declaration to G_DECLARE* (1.85 KB, patch)
2017-08-22 15:08 UTC, Vyas Giridhar
committed Details | Review
icon-info: ported declaration to G_DECLARE* (2.25 KB, patch)
2017-08-22 15:13 UTC, Vyas Giridhar
committed Details | Review
search-engine-model: ported declaration to G_DECLARE* (9.71 KB, patch)
2017-08-22 15:27 UTC, Vyas Giridhar
committed Details | Review
search-engine-tracker: ported declaration to G_DECLARE* (12.19 KB, patch)
2017-08-22 16:07 UTC, Vyas Giridhar
committed Details | Review
special-location-bar: ported declaration to G_DECLARE* (6.23 KB, patch)
2017-08-22 16:17 UTC, Vyas Giridhar
committed Details | Review

Description Mohammed Sadiq 2016-09-21 18:01:17 UTC
porting nautilus code to G_DECLARE*_TYPE shall reduce a a huge number of boilerplate code.

That would make nautilus code better and easier for newcomers and others alike.
Comment 1 Carlos Soriano 2016-09-21 18:07:54 UTC
Yes, however this is a implementation detail, and it's too abstract. Not sure it worth to have a bug report.

Only case I can think of is to assign newcomers tasks, but we already discussed this in the channel and we decided this task was too tedious for newcomers.

So I'm closing as not a bug.
Comment 2 Ernestas Kulik 2016-09-21 18:25:24 UTC
(In reply to Carlos Soriano from comment #1)
> Only case I can think of is to assign newcomers tasks, but we already
> discussed this in the channel and we decided this task was too tedious for
> newcomers.

We did? I only remember you agreeing with me that NautilusFilesView was maybe too much for a newcomer (the newcomer I had in mind was one, who has little coding experience). Otherwise, I think this is great.

> Yes, however this is a implementation detail, and it's too abstract. Not
> sure it worth to have a bug report.

This could be a metabug for tracking (i.e. playing with dependencies, for easier discovery and whatnot).
Comment 3 Carlos Soriano 2016-09-21 19:51:33 UTC
Two against one, you win :) reopening and marking for newcomers.
Comment 4 Vyas Giridhar 2017-08-21 14:57:29 UTC
Created attachment 358077 [details] [review]
list-view: ported declaration to G_DECLARE*

This patch ports declaration of NautilusListView to
the G_DECLARE* format.
Comment 5 Vyas Giridhar 2017-08-21 14:59:33 UTC
Created attachment 358078 [details] [review]
icon-info: ported declaration to G_DECLARE*

This patch ports declaration of NautilusListView to
the G_DECLARE* format.
Comment 6 Vyas Giridhar 2017-08-21 15:20:02 UTC
Created attachment 358079 [details] [review]
icon-info: ported declaration to G_DECLARE*

This patch ports declaration of NautilusIconInfo to
the G_DECLARE* format.
Comment 7 Ernestas Kulik 2017-08-21 15:21:42 UTC
Review of attachment 358077 [details] [review]:

Thanks!

::: src/nautilus-list-view.h
@@ +33,3 @@
 typedef struct NautilusListViewDetails NautilusListViewDetails;
 
+struct _NautilusListView{

Move the struct to the source file. No reason to expose it.

@@ +40,1 @@
 GType nautilus_list_view_get_type (void);

Unnecessary, the G_DECLARE_FINAL_TYPE macro declares this for us.
Comment 8 Ernestas Kulik 2017-08-21 15:24:01 UTC
Review of attachment 358078 [details] [review]:

Looks good, thanks.

::: src/nautilus-icon-info.h
@@ +54,1 @@
 GType    nautilus_icon_info_get_type (void) G_GNUC_CONST;

Not needed.
Comment 9 Ernestas Kulik 2017-08-21 15:26:11 UTC
(In reply to Ernestas Kulik from comment #7)
> Review of attachment 358077 [details] [review] [review]:
> 
> Thanks!
> 
> ::: src/nautilus-list-view.h
> @@ +33,3 @@
>  typedef struct NautilusListViewDetails NautilusListViewDetails;
>  
> +struct _NautilusListView{
> 
> Move the struct to the source file. No reason to expose it.

Scratch this part, let me dig a little deeper.
Comment 10 Vyas Giridhar 2017-08-21 15:27:26 UTC
Created attachment 358080 [details] [review]
search-engine-tracker: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSearchEngineTracker to
the G_DECLARE* format.
Comment 11 Vyas Giridhar 2017-08-21 15:30:57 UTC
Created attachment 358081 [details] [review]
search-engine-model: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSearchEngineModel to
the G_DECLARE* format.
Comment 12 Ernestas Kulik 2017-08-21 15:31:20 UTC
Review of attachment 358077 [details] [review]:

::: src/nautilus-list-view.h
@@ +33,3 @@
 typedef struct NautilusListViewDetails NautilusListViewDetails;
 
+struct _NautilusListView{

My bad, there is reason to expose it. We’ll have to live with it for now.

Do move the brace to a new line, though.
Comment 13 Ernestas Kulik 2017-08-21 15:36:06 UTC
Review of attachment 358080 [details] [review]:

Huzzah!

::: src/nautilus-search-engine-tracker.h
@@ +27,3 @@
 #define NAUTILUS_TYPE_SEARCH_ENGINE_TRACKER		(nautilus_search_engine_tracker_get_type ())
+G_DECLARE_FINAL_TYPE (NautilusSearchEngineTracker, nautilus_search_engine_tracker, NAUTILUS, SEARCH_ENGINE_TRACKER, GObject)
+/*

Forgot your comments there. :p

@@ +36,3 @@
 typedef struct NautilusSearchEngineTrackerDetails NautilusSearchEngineTrackerDetails;
 
+struct _NautilusSearchEngineTracker {

I take it someone is poking the internal state of this one, too? If so, move the brace to a new line.
Comment 14 Vyas Giridhar 2017-08-21 15:36:59 UTC
Created attachment 358082 [details] [review]
special-location-bar: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSpecialLocationBar to
the G_DECLARE* format.
Comment 15 Ernestas Kulik 2017-08-21 15:37:50 UTC
(In reply to Vyas Giridhar from comment #14)
> Created attachment 358082 [details] [review] [review]
> special-location-bar: ported declaration to G_DECLARE*
> 
> This patch ports declaration of NautilusSpecialLocationBar to
> the G_DECLARE* format.

Could you create a new bug for each patch? Thanks.
Comment 16 Ernestas Kulik 2017-08-21 15:38:13 UTC
(In reply to Ernestas Kulik from comment #15)
> (In reply to Vyas Giridhar from comment #14)
> > Created attachment 358082 [details] [review] [review] [review]
> > special-location-bar: ported declaration to G_DECLARE*
> > 
> > This patch ports declaration of NautilusSpecialLocationBar to
> > the G_DECLARE* format.
> 
> Could you create a new bug for each patch? Thanks.

For any new one, I can deal with the current ones being here.
Comment 17 Ernestas Kulik 2017-08-21 15:48:10 UTC
Review of attachment 358082 [details] [review]:

Thanks! We can take it one step further, though. :)

::: src/nautilus-special-location-bar.h
@@ +30,3 @@
 typedef struct NautilusSpecialLocationBarPrivate NautilusSpecialLocationBarPrivate;
 
+struct _NautilusSpecialLocationBar

This one we can definitely move in to the source file. As a bonus, get rid of the NautilusSpecialLocationBarPrivate struct by merging it with this one.

(That also means removing things like g_type_class_add_private()).

@@ +44,1 @@
 GType		 nautilus_special_location_bar_get_type	(void) G_GNUC_CONST;

Unneeded.
Comment 18 Ernestas Kulik 2017-08-21 16:07:29 UTC
Review of attachment 358081 [details] [review]:

This is more less the same as special-location-bar, in that we can clean it up further.

::: src/nautilus-search-engine-model.h
@@ +30,3 @@
 typedef struct NautilusSearchEngineModelDetails NautilusSearchEngineModelDetails;
 
+struct _NautilusSearchEngineModel {

As with special-location-bar, move it to the source file, merge the details struct with this one and get ride of any mention of private data, etc.

@@ +37,1 @@
 GType          nautilus_search_engine_model_get_type  (void);

This is unneeded.
Comment 19 Vyas Giridhar 2017-08-22 14:47:08 UTC
Created attachment 358153 [details] [review]
list-view: ported declaration to G_DECLARE*

This patch ports declaration of NautilusListView to
the G_DECLARE* format.
Comment 20 Vyas Giridhar 2017-08-22 14:55:33 UTC
(In reply to Ernestas Kulik from comment #7)

> @@ +40,1 @@
>  GType nautilus_list_view_get_type (void);
> 
> Unnecessary, the G_DECLARE_FINAL_TYPE macro declares this for us.

It doesn't.
https://github.com/GNOME/glib/blob/master/gobject/gtype.h#L1395
Comment 21 Ernestas Kulik 2017-08-22 14:56:40 UTC
(In reply to Vyas Giridhar from comment #20)
> (In reply to Ernestas Kulik from comment #7)
> 
> > @@ +40,1 @@
> >  GType nautilus_list_view_get_type (void);
> > 
> > Unnecessary, the G_DECLARE_FINAL_TYPE macro declares this for us.
> 
> It doesn't.
> https://github.com/GNOME/glib/blob/master/gobject/gtype.h#L1395

The line after the one you linked to is exactly that.
Comment 22 Vyas Giridhar 2017-08-22 15:02:20 UTC
I will re upload the patches with the changes
Comment 23 Vyas Giridhar 2017-08-22 15:08:23 UTC
Created attachment 358155 [details] [review]
list-view: ported declaration to G_DECLARE*

This patch ports declaration of NautilusListView to
the G_DECLARE* format.
Comment 24 Vyas Giridhar 2017-08-22 15:13:13 UTC
Created attachment 358156 [details] [review]
icon-info: ported declaration to G_DECLARE*

This patch ports declaration of NautilusIconInfo to
the G_DECLARE* format.
Comment 25 Ernestas Kulik 2017-08-22 15:15:25 UTC
Review of attachment 358155 [details] [review]:

Looks fine, thanks.
Comment 26 Ernestas Kulik 2017-08-22 15:18:51 UTC
Review of attachment 358156 [details] [review]:

Yup.
Comment 27 Ernestas Kulik 2017-08-22 15:22:42 UTC
Attachment 358155 [details] pushed as aec6026 - list-view: ported declaration to G_DECLARE*
Attachment 358156 [details] pushed as 2660473 - icon-info: ported declaration to G_DECLARE*
Comment 28 Vyas Giridhar 2017-08-22 15:27:28 UTC
Created attachment 358159 [details] [review]
search-engine-model: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSearchEngineModel to
the G_DECLARE* format.
Comment 29 Ernestas Kulik 2017-08-22 15:43:42 UTC
Review of attachment 358159 [details] [review]:

Looks good.
Comment 30 Vyas Giridhar 2017-08-22 16:07:42 UTC
Created attachment 358168 [details] [review]
search-engine-tracker: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSearchEngineTracker to
the G_DECLARE* format.
Comment 31 Vyas Giridhar 2017-08-22 16:17:57 UTC
Created attachment 358169 [details] [review]
special-location-bar: ported declaration to G_DECLARE*

This patch ports declaration of NautilusSpecialLocationBar to
the G_DECLARE* format.
Comment 32 Ernestas Kulik 2017-08-23 06:27:31 UTC
Review of attachment 358168 [details] [review]:

Sure. I’ll fix up the indentation on my end, no need to waste time.

::: src/nautilus-search-engine-tracker.c
@@ +36,2 @@
 {
+	GObject parent;

We indent using spaces (4 of them).
Comment 33 Ernestas Kulik 2017-08-23 06:30:09 UTC
Review of attachment 358169 [details] [review]:

Thanks!

::: src/nautilus-special-location-bar.c
@@ +29,2 @@
 {
+	GtkInfoBar parent;

Another tab used here.
Comment 34 Ernestas Kulik 2017-08-23 06:35:28 UTC
Attachment 358159 [details] pushed as 5882308 - search-engine-model: ported declaration to G_DECLARE*
Attachment 358168 [details] pushed as ddeca26 - search-engine-tracker: ported declaration to G_DECLARE*
Attachment 358169 [details] pushed as 5898ce2 - special-location-bar: ported declaration to G_DECLARE*
Comment 35 Carlos Soriano 2017-11-05 21:17:14 UTC
All done, we can go home. Thanks Ernestas for keeping with this so much!