Code review comment for lp://qastaging/~attente/libindicator/gicon-serialization

Revision history for this message
Lars Karlitski (larsu) wrote :

Thanks for the patch!

* We need to wait for the new glib before this can land (that's what Jenkins is complaining about).

* Why did you add a changelog entry? Daily releases are created automatically.

* The error checking logic in indicator-image-helper.c is weird. Don't check whether the error is set, but instead check the return value of the function (by convention, @error will be set when functions return NULL or FALSE). Also, use "else" :P

* In indicator_ng_update_entry, you removed the const from 'label' and 'accessible_desc' and freed those strings at the end of the function. That's wrong. They are filled with pointers to the serialized GVariant-data (that's what the '&' in the format string means).

review: Needs Fixing

« Back to merge proposal