GNOME Bugzilla – Bug 764423
Use G_DECLARE_DERIVABLE_TYPE for derivable classes
Last modified: 2017-09-26 14:10:14 UTC
We should switch to using G_DECLARE_DERIVABLE_TYPE for all derivable classes (ie. classes with sub-classes). See https://wiki.gnome.org/HowDoI/SubclassGObject for further details. Please use a separate patch for each class to keep it manageable.
Created attachment 334379 [details] [review] base-item: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 337057 [details] [review] tracker-controller: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 337123 [details] [review] base-manager: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 338296 [details] [review] offset-controller: Use G_DECLARE_DERIVABLE_TYPE In context of bug 764423 - Use G_DECLARE_DERIVABLE_TYPE for derivable classes https://bugzilla.gnome.org/show_bug.cgi?id=764423 photos-offset-controller is derivable so it need to be made derivable
Review of attachment 338296 [details] [review]: Thanks for the patch, Kartikeya! It looks very good. There is no need for the "In context of bug ..." in the commit message. You already have the URL to this bug. See the other patches for examples. ::: src/photos-offset-controller.h @@ +34,2 @@ #define PHOTOS_TYPE_OFFSET_CONTROLLER (photos_offset_controller_get_type ()) Nitpick: extra newline. It should be removed.
Created attachment 338453 [details] [review] offset-controller: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 338458 [details] [review] tool: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 338520 [details] [review] searchbar: Use G_DECLARE_DERIVABLE_TYPE
Created attachment 338526 [details] [review] searchbar: Use G_DECLARE_DERIVABLE_TYPE
Review of attachment 338453 [details] [review]: Perfect!
Review of attachment 338458 [details] [review]: Thanks, Kartikeya.
Review of attachment 338526 [details] [review]: ::: src/photos-searchbar.h @@ +32,2 @@ #define PHOTOS_TYPE_SEARCHBAR (photos_searchbar_get_type ()) +G_DECLARE_DERIVABLE_TYPE (PhotosSearchbar, photos_searchbar, PHOTOS, SEARCHBAR, GtkRevealer) Nitpick: missing semi-colon.
Created attachment 338592 [details] [review] searchbar: Use G_DECLARE_DERIVABLE_TYPE Fixed and pushed.
Created attachment 348486 [details] [review] made photos-search-context derivable photos-search-context: Use G_DECLARE_DERIVABLE_TYPE. In this commit, we are using G_DECLARE_DERIVABLE_TYPE, because photos-search-context has not been further subclassed. https://bugzilla.gnome.org/show_bug.cgi?id=764423
Comment on attachment 348486 [details] [review] made photos-search-context derivable >From ead90ef18c2abbb1dfe93ce82815a6f5202eb0a5 Mon Sep 17 00:00:00 2001 >From: RashiSah <rashi.747@gmail.com> >Date: Wed, 22 Mar 2017 18:59:48 +0530 >Subject: [PATCH] photos-search-context: Use G_DECLARE_DERIVABLE_TYPE > >https://bugzilla.gnome.org/show_bug.cgi?id=764423 >--- > src/photos-search-context.h | 29 +---------------------------- > 1 file changed, 1 insertion(+), 28 deletions(-) > >diff --git a/src/photos-search-context.h b/src/photos-search-context.h >index bd0662a..85f0d21 100644 >--- a/src/photos-search-context.h >+++ b/src/photos-search-context.h >@@ -30,34 +30,8 @@ > G_BEGIN_DECLS > > #define PHOTOS_TYPE_SEARCH_CONTEXT (photos_search_context_get_type ()) >+G_DECLARE_DERIVABLE_TYPE (PhotosSearchContext, photos_search_context, PHOTOS, SEARCH_CONTEXT, gpointer); > >-#define PHOTOS_SEARCH_CONTEXT(obj) \ >- (G_TYPE_CHECK_INSTANCE_CAST ((obj), \ >- PHOTOS_TYPE_SEARCH_CONTEXT, PhotosSearchContext)) >- >-#define PHOTOS_IS_SEARCH_CONTEXT(obj) \ >- (G_TYPE_CHECK_INSTANCE_TYPE ((obj), \ >- PHOTOS_TYPE_SEARCH_CONTEXT)) >- >-#define PHOTOS_SEARCH_CONTEXT_GET_INTERFACE(inst) \ >- (G_TYPE_INSTANCE_GET_INTERFACE ((inst), \ >- PHOTOS_TYPE_SEARCH_CONTEXT, PhotosSearchContextInterface)) >- >-typedef struct _PhotosSearchContextState PhotosSearchContextState; >- >-typedef struct _PhotosSearchContext PhotosSearchContext; >-typedef struct _PhotosSearchContextInterface PhotosSearchContextInterface; >- >-struct _PhotosSearchContextState >-{ >- gpointer item_mngr; >- gpointer mode_cntrlr; >- gpointer src_mngr; >- gpointer srch_mtch_mngr; >- gpointer srch_typ_mngr; >- gpointer offset_cntrlr; >- gpointer srch_cntrlr; >-}; > > PhotosSearchContextState *photos_search_context_state_new (PhotosSearchContext *self); > >@@ -71,7 +45,6 @@ struct _PhotosSearchContextInterface > PhotosSearchContextState *(*get_state) (PhotosSearchContext *self); > }; > >-GType photos_search_context_get_type (void) G_GNUC_CONST; > > PhotosSearchContextState *photos_search_context_get_state (PhotosSearchContext *self); > >-- >2.9.3 >
Created attachment 348509 [details] [review] Made photos-search-type derivable photos-search-type: Use G_DECLARE_DERIVABLE_TYPE.
Review of attachment 348486 [details] [review]: Thanks for the patch. ::: src/photos-search-context.h @@ +32,2 @@ #define PHOTOS_TYPE_SEARCH_CONTEXT (photos_search_context_get_type ()) +G_DECLARE_DERIVABLE_TYPE (PhotosSearchContext, photos_search_context, PHOTOS, SEARCH_CONTEXT, gpointer); PhotosSearchContext is an interface, which is different from a base class. For example, a type can implement multiple interfaces, but can have only one base class. So, we will have to use G_DECLARE_INTERFACE. See commit d629308e815cbe and the ones before it for examples. It might be easier for you to try bug 763712 instead.
Review of attachment 348509 [details] [review]: ::: src/photos-search-type.h @@ +32,2 @@ #define PHOTOS_TYPE_SEARCH_TYPE (photos_search_type_get_type ()) +G_DECLARE_DERIVABLE_TYPE (PhotosSearchtype, photos_search_type, PHOTOS, SEARCH_TYPE, GObject); No, PhotosSearchType shouldn't be derivable because nobody (needs to) derives from it. It should remain a final class. See bug 763712
Created attachment 348529 [details] [review] photos-search-context: Used-G_DECLARE_INTERFACE
Review of attachment 348529 [details] [review]: ::: src/photos-search-context.h @@ +32,2 @@ #define PHOTOS_TYPE_SEARCH_CONTEXT (photos_search_context_get_type ()) +G_DECLARE_INTERFACE (PhotosSearchContext, photos_search_context, PHOTOS, SEARCH_CONTEXT, GObject); This needs to be merged/squashed with your previous patch that touches this same class. See 'git rebase -i', 'git commit --amend' on how to do that.
Squashed with the previous patch Terminal: [detached HEAD dd4e212] search-context: Use G_DECLARE_INTERFACE Date: Wed Mar 22 20:12:49 2017 +0530 2 files changed, 2 insertions(+), 19 deletions(-) Successfully rebased and updated refs/heads/master
Created attachment 348670 [details] [review] tracker-change-monitor: Use G_DECLARE_DERIVABLE_TYPE
(In reply to Rashi from comment #22) > Squashed with the previous patch > Terminal: > [detached HEAD dd4e212] search-context: Use G_DECLARE_INTERFACE > Date: Wed Mar 22 20:12:49 2017 +0530 > 2 files changed, 2 insertions(+), 19 deletions(-) > Successfully rebased and updated refs/heads/master Umm... where is the patch? I don't see it here as an attachment. When you attach, please mark the older versions as obsolete (details -> obsolete).
Review of attachment 348670 [details] [review]: No, TrackerChangeMonitor is not a derivable class. It is a final class. Please read https://wiki.gnome.org/HowDoI/SubclassGObject, as mentioned in the bug, to understand the difference between the two.
Created attachment 348688 [details] [review] search-context: Use G_DECLARE_INTERFACE
Created attachment 349058 [details] [review] search-context: Use G_DECLARE_INTERFACE search-context: Used G_DECLARE_INTERFACE
Review of attachment 349058 [details] [review]: Thanks for fixing the typo, Rashi. Sadly, for various reasons it doesn't build. Does it build for you? Did you study other uses of G_DECLARE_INTERFACE in the gnome-photos code base?
Created attachment 351523 [details] [review] search-context: Use G_DECLARE_INTERFACE