GNOME Bugzilla – Bug 685473
grl_source_query(): Does not hint at the query syntaxes
Last modified: 2012-11-30 08:38:35 UTC
The grl_source_query() documentation http://developer.gnome.org/grilo/unstable/GrlSource.html#grl-source-query just says that the query syntax is plugin-specific. It should give more hints, ideally linking to the relevant documentation for the standard/popular plugins. I think (and jasuarez on irc agrees), that the query syntax should be part of the class overview documentation for the derived GrlPlugin classes in gtk-doc: <jasuarez> Brief description of what it provides, description of the supported operations, description of the supported configuration values, and description of the query syntax.
Created attachment 228444 [details] [review] 0001-build-Use-variables-for-CFLAGS-and-LIBS.patch build: Use variables for CFLAGS and LIBS. * configure.ac: Define variables for the plugin-specific CFLAGS and LIBS. * src/*/Makefile.am: Use the variables here instead of mentioning the CFLAGS and LIBS for each individual dependency. This lets us use the CFLAGS and LIBS elsewhere, such as when using gtk-doc.
Created attachment 228445 [details] [review] 0002-Add-gtk-doc-documentation-generation.patch Add gtk-doc documentation generation * configure.ac, Makefile.am, doc/Makefile.am: Add use of gtk-doc, disabled by default but enabled during dist. * doc/grilo-plugins/: Start with the *.types, and the main *-docs.xml page, with an overview.xml. * src/*/*.c: Add gtk-doc comments to the *Source types so that they appear in the documentation. These should later contain documentation about what full-text search and query syntax each plugin supports, as suggested in bug #685473 * Makefile.am: Do not make docs an optional part of SUBDIRS, because we need to install the distribute html even when the generation of that html is disabled.
Created attachment 228446 [details] [review] 0003-docs-Enable-all-plugins-in-distcheck.patch docs: Enable all plugins in distcheck
Created attachment 228448 [details] [review] 0004-tmdb-Add-documentation.patch tmdb: Add documentation. Also add an example .c file and try to show it in the documentation. Note that, for some reason, I had to use <informalexample> instead of <example> so that it actually shows the example code in the generated HTML.
Here are patches to use gtk-doc generally (disabled by default, and needs all plugins to be enabled when it is used), and a patch to use this to document the tmdb plugin. May I push these to master?
Review of attachment 228445 [details] [review]: ::: src/dmap/grl-dmap.h @@ +48,3 @@ GRL_DMAP_SOURCE_TYPE)) +#define GRL_DMAP_SOURCE_GET_CLASS(obj) \ I'm pushing this specific fix separately
Review of attachment 228448 [details] [review]: ::: examples/example-tmdb.c @@ +57,3 @@ + +int main (int argc, char *argv[]){ + g_type_init (); g_type_init() is deprecated in latest glib version. #if !GLIB_CHECK_VERSION(2,35,0) g_type_init (); #endif
Some comments: - I'm in the process of releasing, so I'm leaving these patches for next release. Also, I'll take the release window to add documenation for the other plugins - Documentation should be generated even if only part of the plugins are enabled. Right now, if some plugin can't be built, the building process fails when gtk-doc is enabled. - We need take care with the example. Though it is possible, we don't want developers to deal directly with the TMDb source; rather the source should be used by the core when asking for property keys that the source providing the information can't solve. - The documenation shouldn't list the source clases directly. When checking the documenation, I see it shows GrlJamendoSource, GrlYoutubeSource, and so on. I think this is wrong, because developers never deals with those clases directly; they only see GrlSource. IMHO, we shouldn't put the documentation in the .c, and rather create a section per plugin, that shouldn't be listed as "API documentation". An example could something like "YouTube plugin", that briefly explains what the plugin does, the plugin id (this is what the developer really needs to know), the source id, configuration keys, supported operations (just a simple list), and so on. But documentation shouldn't list the real gobject classes in the plugin.
(In reply to comment #8) > Some comments: > - Documentation should be generated even if only part of the plugins are > enabled. I don't think that's possible. The .sgml/XML *-doc.sgml file needs to link to the specific files. I guess we could do some weird generation of the file based on the configure result, but it wouldn't be pretty. > Right now, if some plugin can't be built, the building process fails > when gtk-doc is enabled. And I think that's not so bad. The documentation is disabled by default and packagers (who want to not build some plugin) would not build the documentation anyway, because the generated files are distributed.
I accidentally pushed this, but immediately reverted the commits. Sorry.
(In reply to comment #8) > IMHO, we shouldn't put the documentation in the .c, and rather create a section > per plugin, that shouldn't be listed as "API documentation". > An example could something like "YouTube plugin", that briefly explains what > the plugin does, the plugin id (this is what the developer really needs to > know), the source id, configuration keys, supported operations (just a simple > list), and so on. > But documentation shouldn't list the real gobject classes in the plugin. OK, that's why I tried to get some agreement about this beforehand. In this case, is there any benefit to using gtk-doc? We could just put that in DocBook XML files? What would you like?
(In reply to comment #7) > g_type_init() is deprecated in latest glib version. I've fixed that, and some whitespace, in this git branch: http://git.gnome.org/browse/grilo-plugins/log/?h=docs
(In reply to comment #11) > OK, that's why I tried to get some agreement about this beforehand. In this > case, is there any benefit to using gtk-doc? We could just put that in DocBook > XML files? What would you like? I think we could put it in the docbook xml files. As far as I understand, gtk-doc is for the case of API documentation, which is not exactly what I want. So yes, I think docbook xml is what I'm thinking about (or the new mallard system? at the end, we are writting like a user manual, but for the developers)
How about this? http://git.gnome.org/browse/grilo-plugins/commit/?h=docs&id=3dc19128b7b9bb35d71ef5808c6eeb2e4b160eda If that's what you want, I'll git rebase -i away the gtk-doc stuff. Regardless of this, I think you should want the patches in comment #1 and comment #3, just for general cleanup and correctness. You might also want to keep the gtk-doc comments in the .c files, though it would be a duplication.
(In reply to comment #8) > - We need take care with the example. Though it is possible, we don't want > developers to deal directly with the TMDb source; rather the source should be > used by the core when asking for property keys that the source providing the > information can't solve. Is that something that happens automatically already? Can an application enable/disable specific plugins for its own use?
(In reply to comment #14) > How about this? > http://git.gnome.org/browse/grilo-plugins/commit/?h=docs&id=3dc19128b7b9bb35d71ef5808c6eeb2e4b160eda > > If that's what you want, I'll git rebase -i away the gtk-doc stuff. Yes! That is what I had in mind. Could it be possible to split each plugin section in its own file, and put links in the main file? That would make it easier to maintain. Also, just check everything is right. Running here I saw that Vimeo is listed as UPnP plugin. Finally, I think we could try to define a template with kind of information all sources should document, so it makes easier for developers to find the information they need. But let's do this later. > > Regardless of this, I think you should want the patches in comment #1 and > comment #3, just for general cleanup and correctness. > Yes, I think it is a good idea. > > You might also want to keep the gtk-doc comments in the .c files, though it > would be a duplication. If the gtk-doc is just for having references to grilo doc (like when you mention grl_registry_get_default()) then it's fine for me keeping them. Else, we can remove it.
(In reply to comment #15) > (In reply to comment #8) > > - We need take care with the example. Though it is possible, we don't want > > developers to deal directly with the TMDb source; rather the source should be > > used by the core when asking for property keys that the source providing the > > information can't solve. > > Is that something that happens automatically already? Can an application > enable/disable specific plugins for its own use? Dunno if I follow your question... If you are browsing, let's say, YouTube plugin, and you request the PERFORMER key, the TMDb plugin will be used automatically by the core; you don't need to explicitly ask TMDb for the key. And yes, applications and/or users can deactivate plugins/sources as they need: a) Using "--grl-plugin-use" to define the list of plugins to use b) Same above, but using GRL_PLUGIN_LIST envvar c) Using grl_registry_load_plugin_by_id() d) Load all sources, and unregister those not interested
> Could it be possible to split each plugin section in its own file, and put > links in the main file? That would make it easier to maintain. Could I persuade you not to want that? That can be awkward because you then need the same copy/pasted list of XML entitites at the start of each .xml file. For instance, the grilo version, reference API URLs, etc. It's also then generally more awkward to validate the XML manually and to find what you are looking for across multiple files. > Also, just check everything is right. Running here I saw that Vimeo is listed > as UPnP plugin. Fix in the docs branch. > > You might also want to keep the gtk-doc comments in the .c files, though it > > would be a duplication. > > If the gtk-doc is just for having references to grilo doc (like when you > mention grl_registry_get_default()) then it's fine for me keeping them. Else, > we can remove it. No, that's just a simple link to the website. gtk-doc is not involved. So I will not put that commit in master.
Ok, go ahead. We can merge that branch in master. Thank you.
Pushed to master, without the gtk-doc stuff. Thanks. It would be great to get this in a tarball release, so we can show it on developer.gnome.org.
Created attachment 230218 [details] [review] Last day I was running in problems with the documentation while executing "make distcheck". Finally it turned out that moving a line in doc/Makefile.am (see commit 007a83d9e) fixed the problem. But while trying to fix it, I found this link about how to use the GNOME Documentation tools: http://developer.gnome.org/gnome-doc-make/unstable/migrating.html Some of the suggestions were already done, but others not. So the attached commit adds some suggestions, specifically: - Adds legal.xml file - Disables scrollkeeper from distcheck flags - Adds gnome-doc-utils.make to distclean - Renames doc/ to help/ Probably the biggest change is renaming from doc/ to help/. I was checking how other modules do in GNOME, and all of them uses help to store this kind of documentation. Granted, most of them are applications, and we are a library; but I think this is irrelevant, because this documentation is provided as a "handbook" that developers can check to know more details about the plugins. It doesn't provide API documentation at all. I've checked and it works fine. But I'm attaching here the commit just in case you want to take a look. If you are fine, I'll merge this commit in master and do a new release.
(In reply to comment #21) > Created an attachment (id=230218) [details] [review] > Last day I was running in problems with the documentation while executing "make > distcheck". > > Finally it turned out that moving a line in doc/Makefile.am (see commit > 007a83d9e) fixed the problem. Maybe you forgot to push that. I don't see it here: http://git.gnome.org/browse/grilo-plugins/log/doc/Makefile.am > If you are fine, I'll merge this commit in master and do a new release. Yes, please. Consistency is good. Thanks.
(In reply to comment #22) > > Finally it turned out that moving a line in doc/Makefile.am (see commit > > 007a83d9e) fixed the problem. > > Maybe you forgot to push that. I don't see it here: > http://git.gnome.org/browse/grilo-plugins/log/doc/Makefile.am > Ops! Yes, seems I didn't push it. > > If you are fine, I'll merge this commit in master and do a new release. > > Yes, please. Consistency is good. Thanks. Great!. I'll merge & push and prepare the new release.