GNOME Bugzilla – Bug 768380
libjson-glib-1.0: Mark serializable.find_property as nullable
Last modified: 2016-07-21 14:15:44 UTC
Created attachment 330852 [details] [review] Patch adding nullable metadata to 'Serializable.find_property' As stated in the documentation: > the ParamSpec for the property or null if no property was found
Review of attachment 330852 [details] [review]: Would it be better if the GObject Introspection annotation for json_serializable_find_property in https://github.com/ebassi/json-glib/blob/master/json-glib/json-serializable.c was changed from * Return value: (transfer none): the #GParamSpec for the property to * Return value: (transfer none) (nullable): the #GParamSpec for the property Otherwise patch looks fine, although extra white space before .deserialize_property.value could be removed to maintain alignment.
(In reply to Al Thomas from comment #1) > Review of attachment 330852 [details] [review] [review]: > > Would it be better if the GObject Introspection annotation for > json_serializable_find_property in > https://github.com/ebassi/json-glib/blob/master/json-glib/json-serializable. > c was changed from > > * Return value: (transfer none): the #GParamSpec for the property > > to > > * Return value: (transfer none) (nullable): the #GParamSpec for the property > > Otherwise patch looks fine, although extra white space before > .deserialize_property.value could be removed to maintain alignment. Sure, but that would require an upstream fix and two releases (us and them) since Vala ships the binding. I might have other fixes too for that metadata file, so I'll include them here as well.
Created attachment 330853 [details] [review] Patch missing out parameter on 'Serializable.default_deserialize_property'
Review of attachment 330852 [details] [review]: Updated patch submitted
Review of attachment 330853 [details] [review]: The annotation for json_serializable_default_deserialize_property: * @value: a pointer to an uninitialized #GValue So an out parameter is right. Unfortunately this patch loses - public abstract unowned GLib.ParamSpec find_property (string name); + public abstract unowned GLib.ParamSpec? find_property (string name); from the previous patch, although + .find_property nullable is included in the new patch.
(In reply to Al Thomas from comment #5) > Review of attachment 330853 [details] [review] [review]: > > The annotation for json_serializable_default_deserialize_property: > > * @value: a pointer to an uninitialized #GValue > > So an out parameter is right. > Unfortunately this patch loses > - public abstract unowned GLib.ParamSpec find_property (string name); > + public abstract unowned GLib.ParamSpec? find_property (string name); > from the previous patch, although > + .find_property nullable > is included in the new patch. I'll squash them and propose a single patch instead.
Created attachment 330863 [details] [review] Squashed the two previously proposed patches
Review of attachment 330863 [details] [review]: Looks good. Comments above have been addressed.