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 777607 - canvas-view: port from old-style type declarations
canvas-view: port from old-style type declarations
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: general
master
Other Linux
: Normal minor
: ---
Assigned To: Rohit Kaushik
Nautilus Maintainers
Depends on:
Blocks: 771777
 
 
Reported: 2017-01-22 14:25 UTC by Ernestas Kulik
Modified: 2017-02-03 13:54 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
canvas-view: port to G_DECLARE* type declaration (16.54 KB, patch)
2017-01-30 12:41 UTC, Rohit Kaushik
none Details | Review
canvas-view: port to G_DECLARE* type declaration (16.70 KB, patch)
2017-01-30 19:54 UTC, Rohit Kaushik
none Details | Review
canvas-view: port to G_DECLARE* type declaration (16.45 KB, patch)
2017-01-30 19:58 UTC, Rohit Kaushik
committed Details | Review

Description Ernestas Kulik 2017-01-22 14:25:18 UTC
See bug 771777.
Comment 1 Rohit Kaushik 2017-01-30 12:41:39 UTC
Created attachment 344534 [details] [review]
canvas-view: port to G_DECLARE* type declaration

This patch reduces the manual definition of macros and ports it
to G_Declare type declaration, which make the code cleaner and
easier to read and understand.

The NautilusCanvasDetails has been renamed to NautilusCanvasViewPrivate.
Every instance of canvas_view->details has been modified to priv (reference to
the private Structure)
Comment 2 Ernestas Kulik 2017-01-30 19:33:43 UTC
Review of attachment 344534 [details] [review]:

Cool, thanks!

As I mentioned last time, I’m paying more attention to the commit message.

> The NautilusCanvasDetails has been renamed to NautilusCanvasViewPrivate.
> Every instance of canvas_view->details has been modified to priv (reference to
> the private Structure)

This is unnecessary, since the diff already tells us that.
Take a look at the tips section: https://wiki.gnome.org/Newcomers/CodeContributionWorkflow#Commit_guidelines

I think the first paragraph alone will do.

::: src/nautilus-canvas-view.c
@@ +1969,2 @@
 static void
 nautilus_canvas_view_finalize (GObject *object)

Since we’re only chaining up to the parent method, this no longer needed (don’t forget the override as well).

::: src/nautilus-canvas-view.h
@@ +43,2 @@
 int     nautilus_canvas_view_compare_files (NautilusCanvasView   *canvas_view,
+					    NautilusFile *a,

Please do style changes separately. Attach another patch for these.
Comment 3 Rohit Kaushik 2017-01-30 19:54:02 UTC
Created attachment 344577 [details] [review]
canvas-view: port to G_DECLARE* type declaration

This patch reduces the manual definition of macros and ports it
to G_Declare type declaration, which make the code cleaner and
easier to read and understand.
Comment 4 Rohit Kaushik 2017-01-30 19:58:05 UTC
Created attachment 344578 [details] [review]
canvas-view: port to G_DECLARE* type declaration

This patch reduces the manual definition of macros and ports it
to G_Declare type declaration, which make the code cleaner and
easier to read and understand.
Comment 5 Ernestas Kulik 2017-01-31 17:08:02 UTC
Review of attachment 344578 [details] [review]:

LGTM, thanks. :)
Comment 6 Carlos Soriano 2017-02-02 20:09:31 UTC
Review of attachment 344578 [details] [review]:

yep!
Comment 7 Ernestas Kulik 2017-02-03 13:54:04 UTC
Attachment 344578 [details] pushed as 04811bc - canvas-view: port to G_DECLARE* type declaration