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 <[email protected]> > 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 > > >
gegl-processor.diff
Description: Binary data
_______________________________________________ Gegl-developer mailing list [email protected] https://lists.XCF.Berkeley.EDU/mailman/listinfo/gegl-developer
