Re: [gnome-db] New patch



On Thu, Jan 10, 2002 at 02:07:46PM +0100, Rodrigo Moya wrote:
> some things are wrong. You've added const to the GdaField parameter in
> the get_ functions. There is no need for doing so. What you should do is
> to add const for the return values where appropriate, but not to the
> GdaField paramerter. What is also const are the second arguments to the
> set_ functions, but only for pointers. There's no need for doing 'const
> gint' or something similar.

	Ok.
> 
> Also:
> 
> > -GDate *
> > -gda_field_get_date_value (GdaField *field)
> > +const GdaDate *
> > +gda_field_get_date_value (const GdaField *field)
> >  {
> >         g_return_val_if_fail (field != NULL, NULL);
> > -       g_return_val_if_fail (
> > -               field->attributes.gdaType == GDA_TYPE_DATE,
> > -               NULL);
> > 
> >         return gda_value_get_date (&field->value);
> >
> why are you removing that check?

	Because now in gda_value_get_date() It is already done:

return BONOBO_ARG_GET_GENERAL (value, GDA_VALUE_TYPE_DATE, GdaDate *,
                                        NULL);

	BONOBO_ARG_GET_GENERAL does it for me. The same for
	gda_field_get_time_value() and *timestamp_value().

	Do you still want me to put those lines back?

> > 	gda-value.c still has a few warnings related to ORBit functions
> > 	(ORBit_RootObject_duplicate() and CORBA_Object_release()).
> > 	Somebody, please, take a look at them.
> >
> this part looks good, except for the extra const you've added in some
> places.
> I've only applied the idl part, so please resend the libgda part. The
> postgres and test parts look good, but haven't applied them as they need
> the changes in libgda. So please resend that part (along with the
> postgres and testing patches), and if it is ok, you can apply yourself
> with your new CVS account :-)

	Ok. So I'll remove all const inserted except those for return
	values that are pointers.

>  
> > 	My TODO list:
> > 
> > 		-gda-test: display the GdaRecordset contents (don't know
> > 		how!).
> > 
> in testing/model.c there's a function (display_data_model or something
> like that which will do it for you). Also, the schemas function in
> client.c contain some code you can use as example.
> BTW, could you also move the postgres-specific parts in client.c to a
> separate file (postgres.c)? This will make the code cleaner, and then we
> can add new files for provider specific tests.

	Yes, I also think it is a good idea to make a separate file for
	specific tests. I'll do.

	Give me a few hours and I'll be back with the corrected patch
	:-).

	Thanks for your time!




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]