Re: [PATCH] add graph window

[Christian Neumair <chris gnome-de org>]

Thanks again for your review.

On Fr, 2005-11-18 at 00:51 -0500, Jody Goldberg wrote:
> > +GogObject   *gog_object_get_child_by_name(GogObject const *obj, const char *name);
> please commit this with the minor stylistic adjustment that in
> gnumeric/goffice we use
>     char const *
> in place of
>     const char *
Done.


> > --- goffice/gtk/Makefile.am 5 Sep 2005 21:02:55 -0000       1.30
> > +++ goffice/gtk/Makefile.am 4 Nov 2005 11:46:36 -0000
> > -   go-graph-widget.c
> > +   go-graph-widget.c \
> > +   \
> > +   go-marshal.list
> Please keep these in goffice/utils (...).

Turns out that we didn't need it.
 
> > +           static const GEnumValue values [] = {
> > +                   { GO_GRAPH_WIDGET_REFERENCE_DIRECTION_NONE, 
> > "GO_GRAPH_WIDGET_REFERENCE_DIRECTION_NONE", "horizontal" },
> This looks wrong.  Shouldn't that be "none" ?

Fixed.

> > +                (w->zoom_factor > 1.0 ?
> > +                 w->height < w->width * w->aspect_ratio :
> > +                 w->height > w->width * w->aspect_ratio))) {
> What are you trying to achieve here ?

I've explained my rationale in the other email [1].

> >  /**
> > + * go_graph_widget_set_graph :
> > + * @widget : #GOGraphWidget
> > + * @graph: #GogGraph
> > + * 
> > + * Sets the #GogGraph embedded in the widget. May not be NULL.
> Please add a comment that it adds a reference.  To me the most
> important docs are always lifecycle related. (...)
> ditto (for go_graph_widget_get_chart).

Done.

> > --- goffice/gtk/go-graph-widget.h   8 Aug 2005 08:57:00 -0000       1.2
> > +++ goffice/gtk/go-graph-widget.h   4 Nov 2005 11:46:37 -0000
> > @@ -32,12 +32,23 @@ G_BEGIN_DECLS
> > +typedef struct _GOGraphWidgetClass GOGraphWidgetClass;
> Why make this public ?
Fixed.

Reattaching patch.

-- 
Christian Neumair <chris gnome-de org>

Attachment: goffice-graph-widget-5.diff
Description: Text Data

Attachment: signature.asc
Description: This is a digitally signed message part