Re: [BRANCHES] Giving some sanity to dates

[Guillaume Emont <guijemont igalia com>]

On 27/09/2011 17:15, Juan A. Suarez Romero wrote:
> On Tue, 2011-09-27 at 14:40 +0200, Guillaume Emont wrote:
>> Hmm, it's been a while ago. I probably made it like that out of fear
>> that _set_modification_date() gets called while we are using the value
>> returned by the getter without taking a reference.
>> But arguably, there's the same issue with all the getters of GrlMedia,
>> and if we document that the returned value remains owned by the
>> GrlMedia, people should expect to have to handle that properly. I'm OK
>> to change that (seems to work with bindings too) for consistency.
> 
> Yes, well. The point is not only to know if returned data belongs to the
> GrlMedia or not, but also to know if user must free it or not.
> 

Sorry, I might not have been clear: when I said "a belongs to b" I meant
"b holds the current reference on a". I meant that only for reference
counting purposes, not for whether modification is possible (in the case
of GDateTime, no modification is possible, the type is unmutable).

> In the case of returning a string, it's pretty clear from the signature:
> the string prefixed with 'const' belongs to GrlMedia and thus shouldn't
> be freed.
> 
> I really don't know if returning a GDataTime it must be prefixed it
> 'const' to tell user that the data belongs GrlMedia. If we do it,
> clearly it is a pain later to deal with it.
Yeah, we just can't return a const because all the functions you want to
use with a GrlMedia take non-const arguments.
> 
> But either if we add the 'const' or not, I would like to see a
> consistency, and that's why I'm proposing to return it unreffed.
> 
> Any other opinion?
> 
> 	J.A.
> 
> PD: Yes it's my full review. Other than those nitpicks, everything is
> fine for me.
Ok. Have you had a look at the updated branch where these issues have
been addressed? Are you OK with them? Am I good to push?

Guij
> 
> 
> _______________________________________________
> grilo-list mailing list
> grilo-list gnome org
> http://mail.gnome.org/mailman/listinfo/grilo-list
>