[Gegl-developer] Small patch

[Henrik Akesson]

Thank you for the comments, the patch has been updated accordingly.

One question: sometimes I see some methods without the gegl_something_
prefix, does that signify something (like private method)?

Second question: what does BLIT mean?

Henrik

2009/3/12 Sven Neumann <sven at gimp.org>

> Hi,
>
> some comments on your patch:
>
>
> +#include <glib.h>
>  #include <glib-object.h>
>
> That's redundant as glib-object.h already includes this file for you.
>
>
> +inline gint      get_band_size               (gint  size);
>
> Splitting the code into smaller functions is cool. But please consider
> to name the functions according to the file it lives in. So this would
> become gegl_processor_get_band_size(). This makes it easier to interpret
> stack traces and to discuss the code as it is clear where the function
> is located.
>
>
> +inline gint
> +get_band_size ( gint size )
>
> Please format this as get_band_size (gint size). Also you will want to
> declare this function as static. And as far as I can see it should also
> be labeled G_GNUC_CONST as it doesn't examine any values except its
> parameters, and has no effects except its return value. I wouldn't
> declare it inline though. The compiler can better decide if it makes
> sense to inline this function.
>
>
> I am not happy about the way you reformatted the code in the class_init
> method:
>
> -  g_object_class_install_property (gobject_class, PROP_NODE,
> -                                   g_param_spec_object ("node",
> -                                                        "GeglNode",
> -                                                        "The GeglNode
> to process (will saturate the provider's cache if the provided node is a
> sink node)",
> -                                                        GEGL_TYPE_NODE,
> -
> G_PARAM_WRITABLE |
> -
> G_PARAM_CONSTRUCT));
> +  g_object_class_install_property (
> +    gobject_class,
> +    PROP_NODE,
> +    g_param_spec_object (   "node",
> +                            "GeglNode",
> +                            "The GeglNode to process (will saturate the
> provider's "
> +                            "cache if the provided node is a sink
> node)",
> +                            GEGL_TYPE_NODE,
> +                            G_PARAM_WRITABLE |
> +                            G_PARAM_CONSTRUCT));
>
> Can we please keep the formatting here? If you want to reduce the code
> to fewer columns, you could break the blurb into multiple lines.
>
>
> Other than that, the patch looks good to me. If you incorporate my
> suggestions I will take care of getting the next version into trunk.
> Nice to see some work being done here.
>
>
> Sven
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: /lists/gegl-developer/attachments/20090312/80e70b23/attachment-0001.html 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gegl-processor.diff
Type: application/octet-stream
Size: 10888 bytes
Desc: not available
Url : /lists/gegl-developer/attachments/20090312/80e70b23/attachment-0001.obj