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 707642 - Missing direction annotations in TotemPlPlaylist
Missing direction annotations in TotemPlPlaylist
Status: RESOLVED FIXED
Product: totem-pl-parser
Classification: Core
Component: General
unspecified
Other All
: Normal normal
: ---
Assigned To: totem-pl-parser-maint
totem-pl-parser-maint
Depends on:
Blocks:
 
 
Reported: 2013-09-06 16:25 UTC by Arnel Borja
Modified: 2013-09-23 06:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Add totem-pl-playlist.c to plparser_sources (1.21 KB, patch)
2013-09-06 16:25 UTC, Arnel Borja
committed Details | Review
plparse: Add missing direction annotations to TotemPlPlaylist (2.99 KB, patch)
2013-09-06 16:25 UTC, Arnel Borja
committed Details | Review

Description Arnel Borja 2013-09-06 16:25:44 UTC
The functions append, prepend and others in plparse/totem-pl-playlist.c need (in) and (out) annotations for the iter parameters. Also, the file is not being parsed by g-ir-scanner, making the other annotations for this file useless.
Comment 1 Arnel Borja 2013-09-06 16:25:46 UTC
Created attachment 254274 [details] [review]
build: Add totem-pl-playlist.c to plparser_sources

The source file totem-pl-playlist.c is not being parsed by g-ir-scanner
because it is missing in instrospection_sources. This will implicitly
add it to that variable.
Comment 2 Arnel Borja 2013-09-06 16:25:59 UTC
Created attachment 254275 [details] [review]
plparse: Add missing direction annotations to TotemPlPlaylist
Comment 3 Bastien Nocera 2013-09-06 17:12:26 UTC
Review of attachment 254274 [details] [review]:

Fine.
Comment 4 Bastien Nocera 2013-09-06 17:13:35 UTC
Review of attachment 254275 [details] [review]:

That looks fine. Did you test with anything?
Comment 5 Arnel Borja 2013-09-07 05:11:46 UTC
Thanks for the fast review :D

(In reply to comment #4)
> That looks fine. Did you test with anything?

Some of it, especially append. We need this for gnome-music playlists, and the branch for it is in:

https://github.com/gnome-prototypes-team/gnome-music/tree/playlists

I'll commit them later, I'll try to do more tests and might add more patches to this bug.

By the way, is totem-pl-parser going to have a release on 3.9.92? It would be great if there would be, so that we could have playlists on the next release :D
Comment 6 Bastien Nocera 2013-09-07 15:50:19 UTC
(In reply to comment #5)
> Thanks for the fast review :D
> 
> (In reply to comment #4)
> > That looks fine. Did you test with anything?
> 
> Some of it, especially append. We need this for gnome-music playlists, and the
> branch for it is in:
> 
> https://github.com/gnome-prototypes-team/gnome-music/tree/playlists
> 
> I'll commit them later, I'll try to do more tests and might add more patches to
> this bug.

Great.

> By the way, is totem-pl-parser going to have a release on 3.9.92? It would be
> great if there would be, so that we could have playlists on the next release :D

If either Philip or I get time, yes.
Comment 7 Arnel Borja 2013-09-09 03:02:44 UTC
After testing with JavaScript, I found out that more changes are needed to make it work there. There is no iter copy function that GJS needs.

But I think adding a new function is too late for 3.10, and the API/ABI break is already in effect. Also, although I haven't found any other packages dependent on the gobject-introspection of totem-pl-parser yet, these changes might break something.

Also, this will increase gnome-music dependencies - we are trying to be compatible with 3.8 during this cycle. All backwards compatibility will be dropped next cycle. The C-like approach works in Python.

So I decided that I'll just do the other necessary changes and attach new patches here on the next cycle, 3.12.
Comment 8 Bastien Nocera 2013-09-23 06:36:19 UTC
Attachment 254274 [details] pushed as 56f9ffa - build: Add totem-pl-playlist.c to plparser_sources
Attachment 254275 [details] pushed as d35e1d3 - plparse: Add missing direction annotations to TotemPlPlaylist