GNOME Bugzilla – Bug 754308
Code cleanup: break grid_element_set_details into smaller functions
Last modified: 2016-11-08 03:00:31 UTC
Created attachment 310300 [details] [review] Break grid_element_set_details into smaller functions This is purely code cleanup, there are no (intentional) changes to any strings or behavior. I realize this is a bit of a lengthy patch, let me know if you'd prefer it in smaller chunks.
Review of attachment 310300 [details] [review]: Looks good to me, thanks! ::: src/disks/gduvolumegrid.c @@ +1539,3 @@ + result = (jobs != NULL); + g_list_foreach (jobs, (GFunc) g_object_unref, NULL); + g_list_free (jobs); We can clean this up more, using g_list_free_full. @@ +1684,1 @@ + g_assert_nonnull (partition); This macro is only intended for use in test cases! You could use g_assert (partition != NULL), or just omit it. @@ +1713,3 @@ + used after calling, and the result should be freed with g_free. */ +static gchar * +consume_array_as_multiline_string (GPtrArray *lines) I half this this would be cleaner if it wasn't split into its own function. Consider that it's not clear that the GPtrArray needs to have its free function set, or that NULL will be appended to the array. But I think it's fine this way too.
Created attachment 310317 [details] [review] Updated patch
Comment on attachment 310317 [details] [review] Updated patch Seems good, thanks. I committed some code cleanups that I really shouldn't have before releasing 3.17.91 yesterday. Now that that's done, it's really time to stop committing code cleanups, simply because if we're unlucky and this breaks something, it's unlikely to be noticed during the week between 3.17.92 and 3.18.0.
Thanks for committing. Is it OK to put up additional cleanup patches in bugzilla with the understanding that (if accepted) they would not be committed until after the freeze ends?
Yes, please. :) Probably easier if you attach them all to this bug.
Better late than never