After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 740186 - Modernize vala bindings
Modernize vala bindings
Status: RESOLVED FIXED
Product: grilo
Classification: Other
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: grilo-maint
grilo-maint
Depends on:
Blocks: 740184
 
 
Reported: 2014-11-15 18:45 UTC by Marc-Andre Lureau
Modified: 2014-11-27 16:34 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gir: add more annotations (4.67 KB, patch)
2014-11-15 18:46 UTC, Marc-Andre Lureau
reviewed Details | Review
net: keep a reference on cancellable (1.08 KB, patch)
2014-11-15 18:46 UTC, Marc-Andre Lureau
committed Details | Review
vala: modernize bindings generation (16.21 KB, patch)
2014-11-15 18:46 UTC, Marc-Andre Lureau
committed Details | Review
core: define C include for GIR (900 bytes, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review
net: define C include for GIR (919 bytes, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review
core: remove wrong (type uint) annotations for errors (2.19 KB, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review
core: annotate nullable error in callbacks (2.33 KB, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review
net: add GIR annotations for grl_net_wc_request_finish() (1.10 KB, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review
core: annotate ownership transfer of grl_registry_register_source() (1002 bytes, patch)
2014-11-17 11:19 UTC, Marc-Andre Lureau
committed Details | Review

Description Marc-Andre Lureau 2014-11-15 18:45:59 UTC
This is the updated bindings that allow me to build the TuneIn plugin
from bug 740184.
Comment 1 Marc-Andre Lureau 2014-11-15 18:46:02 UTC
Created attachment 290766 [details] [review]
gir: add more annotations
Comment 2 Marc-Andre Lureau 2014-11-15 18:46:06 UTC
Created attachment 290767 [details] [review]
net: keep a reference on cancellable

The caller may unref the cancellable, but the async should not crash
Comment 3 Marc-Andre Lureau 2014-11-15 18:46:11 UTC
Created attachment 290768 [details] [review]
vala: modernize bindings generation

Use GIR and modern autotools helpers.  This binding allows to build
simple plugin, it hasn't been tested for applications.
Comment 4 Bastien Nocera 2014-11-16 01:42:02 UTC
Review of attachment 290766 [details] [review]:

Can you split out the fixes to the grl-net and the fixes to the main library?

(With the comments, that would be one commit for grl-net, and 2 for the main library)

::: src/grl-source.h
@@ +134,3 @@
  * @media: (transfer full): a data transfer object
  * @user_data: user data passed to grl_source_resolve()
+ * @error: (nullable): possible #GError generated at processing

Those are really separate bugs, can you do this in 2 steps? (type uint) is clearly incorrect, remove it in one commit, add the (nullable) in a separate one?
Comment 5 Bastien Nocera 2014-11-16 01:42:57 UTC
Review of attachment 290767 [details] [review]:

Please explain what bug this fixes in the subject line, rather than in the commit body. Looks fine otherwise.
Comment 6 Bastien Nocera 2014-11-16 01:44:43 UTC
Review of attachment 290768 [details] [review]:

Globally, that looks fine to me, but I don't know enough about the "state of the art" vala bindings to comment on this.

(I usually prefer vala to work "out-of-the-box" with the Gir bindings)
Comment 7 Marc-Andre Lureau 2014-11-17 11:19:19 UTC
Created attachment 290847 [details] [review]
core: define C include for GIR
Comment 8 Marc-Andre Lureau 2014-11-17 11:19:27 UTC
Created attachment 290848 [details] [review]
net: define C include for GIR
Comment 9 Marc-Andre Lureau 2014-11-17 11:19:32 UTC
Created attachment 290849 [details] [review]
core: remove wrong (type uint) annotations for errors

For some reason, those annotations where added since daebdc455.
Today GIR seems to deal with callbacks errors fine, so let's remove it
Comment 10 Marc-Andre Lureau 2014-11-17 11:19:39 UTC
Created attachment 290850 [details] [review]
core: annotate nullable error in callbacks

Since those arguments are optional error given to callbacks, annotate
them as nullable.
Comment 11 Marc-Andre Lureau 2014-11-17 11:19:45 UTC
Created attachment 290851 [details] [review]
net: add GIR annotations for grl_net_wc_request_finish()

Annotate out arguments and the memory ownership.
Comment 12 Marc-Andre Lureau 2014-11-17 11:19:51 UTC
Created attachment 290852 [details] [review]
core: annotate ownership transfer of grl_registry_register_source()

Do as the comment in the function says, "Take ownership of the source".
Comment 13 Bastien Nocera 2014-11-17 12:00:28 UTC
Review of attachment 290847 [details] [review]:

Looks good.
Comment 14 Bastien Nocera 2014-11-17 12:01:08 UTC
Review of attachment 290848 [details] [review]:

Yep.
Comment 15 Bastien Nocera 2014-11-17 12:02:10 UTC
Review of attachment 290849 [details] [review]:

Yes.
Comment 16 Bastien Nocera 2014-11-17 12:03:59 UTC
Review of attachment 290850 [details] [review]:

Looks good.
Comment 17 Bastien Nocera 2014-11-17 12:12:46 UTC
Review of attachment 290851 [details] [review]:

Looks good.
Comment 18 Bastien Nocera 2014-11-17 12:13:58 UTC
Review of attachment 290852 [details] [review]:

Sure.
Comment 19 Marc-Andre Lureau 2014-11-17 12:48:00 UTC
What about vala bindings, I guess it is ok too?
Comment 20 Bastien Nocera 2014-11-17 13:06:27 UTC
(In reply to comment #19)
> What about vala bindings, I guess it is ok too?

Best for jasuarez to check them out, try and ping him on IRC.
Comment 21 Marc-Andre Lureau 2014-11-17 13:58:18 UTC
Attachment 290852 [details] pushed as 9621ecd - core: annotate ownership transfer of grl_registry_register_source()
Comment 22 Víctor Manuel Jáquez Leal 2014-11-17 19:41:46 UTC
LGTM!

Thanks Marc-Andre, the vala bindings really needed some love :)
Comment 23 Bastien Nocera 2014-11-27 15:05:44 UTC
Attachment 290768 [details] pushed as 689bf70 - vala: modernize bindings generation
Comment 24 Marc-Andre Lureau 2014-11-27 16:34:19 UTC
I forgot to say you need git version of vapigen (0.26.0-45-ge97c9f5).