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 740814 - "make check" should ensure that every symbol is documented
"make check" should ensure that every symbol is documented
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: docs
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-11-27 17:24 UTC by Xavier Claessens
Modified: 2018-05-24 17:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Doc: Verify at "make check" that all symbols are documented (17.21 KB, patch)
2014-11-27 17:25 UTC, Xavier Claessens
none Details | Review
Doc: Verify at "make check" that all symbols are documented (17.18 KB, patch)
2014-11-27 17:41 UTC, Xavier Claessens
needs-work Details | Review
Doc: glib: Fix all undocumented/unused/undeclared symbols (15.03 KB, patch)
2014-11-29 13:45 UTC, Xavier Claessens
none Details | Review
Doc: gobject: Fix all undocumented/unused/undeclared symbols (12.49 KB, patch)
2014-11-29 13:46 UTC, Xavier Claessens
none Details | Review
Doc: gio: Fix all undocumented/unused/undeclared symbols (38.53 KB, patch)
2014-11-29 13:46 UTC, Xavier Claessens
none Details | Review
Doc: enable gtk-doc checks on linux (902 bytes, patch)
2014-11-29 13:46 UTC, Xavier Claessens
none Details | Review
Doc: glib: Fix all undocumented/unused/undeclared symbols (15.12 KB, patch)
2014-12-08 19:49 UTC, Xavier Claessens
committed Details | Review
Doc: gobject: Fix all undocumented/unused/undeclared symbols (11.72 KB, patch)
2014-12-08 19:49 UTC, Xavier Claessens
needs-work Details | Review
Doc: gio: Fix all undocumented/unused/undeclared symbols (39.20 KB, patch)
2014-12-08 19:49 UTC, Xavier Claessens
none Details | Review
Doc: enable gtk-doc checks on linux (902 bytes, patch)
2014-12-08 19:49 UTC, Xavier Claessens
rejected Details | Review
fixup! Doc: glib: Fix all undocumented/unused/undeclared symbols (2.82 KB, patch)
2014-12-10 15:39 UTC, Xavier Claessens
committed Details | Review
fixup! Doc: gobject: Fix all undocumented/unused/undeclared symbols (37.42 KB, patch)
2014-12-10 15:39 UTC, Xavier Claessens
none Details | Review
Doc: enable gtkdoc-check on linux (1.45 KB, patch)
2014-12-12 16:37 UTC, Xavier Claessens
none Details | Review
Doc: gobject: Fix all undocumented/unused/undeclared symbols (40.72 KB, patch)
2014-12-12 18:04 UTC, Xavier Claessens
none Details | Review
Doc: gio: Fix all undocumented/unused/undeclared symbols (40.32 KB, patch)
2014-12-15 18:57 UTC, Xavier Claessens
none Details | Review
configure.ac: Add ENABLE_GTK_DOC_CHECK conditional (811 bytes, patch)
2014-12-15 18:57 UTC, Xavier Claessens
none Details | Review
Doc: gobject: enable gtkdoc-check (900 bytes, patch)
2014-12-15 18:58 UTC, Xavier Claessens
none Details | Review
Doc: glib: enable gtkdoc-check (879 bytes, patch)
2014-12-15 18:58 UTC, Xavier Claessens
none Details | Review
Doc: gio: enable gtkdoc-check (872 bytes, patch)
2014-12-15 18:58 UTC, Xavier Claessens
none Details | Review

Description Xavier Claessens 2014-11-27 17:24:53 UTC
It is really common to forget to add new symbols in the doc, I think "make check" should fail if we don't have 100% doc coverage. telepathy-glib does that.
Comment 1 Xavier Claessens 2014-11-27 17:25:14 UTC
Created attachment 291671 [details] [review]
Doc: Verify at "make check" that all symbols are documented
Comment 2 Xavier Claessens 2014-11-27 17:25:51 UTC
The above patch does it for glib. The same work should be done for gobject and gio.
Comment 3 Xavier Claessens 2014-11-27 17:41:03 UTC
Created attachment 291673 [details] [review]
Doc: Verify at "make check" that all symbols are documented
Comment 4 Allison Karlitskaya (desrt) 2014-11-28 14:55:27 UTC
Review of attachment 291673 [details] [review]:

hi.

Thanks for undertaking this work but there are a few problems here.

The first is that the patches to fix the docs ought to be split out.

The second is that this feature should not be implemented inside of GLib but rather inside of gtk-doc as something that be switched on by modules that are interested in using it.

The third problem is that I'm not even sure that we can do that.  The list of symbols that end up being found by gtk-doc tends to change with the options and platform that we build GLib with.  I fear that we may end up in a situation where we permanently break 'make check' on every platform other than Linux...
Comment 5 Xavier Claessens 2014-11-28 15:53:06 UTC
(In reply to comment #4)
> Review of attachment 291673 [details] [review]:
> The first is that the patches to fix the docs ought to be split out.

Right, I can do that.

> The second is that this feature should not be implemented inside of GLib but
> rather inside of gtk-doc as something that be switched on by modules that are
> interested in using it.

Right, can be moved there but I never looked at gtkdoc code. The advantage of doing it like this is that it's a trivial copy/paste from tp-glib. Can be moved to gtkdoc later if someone cares, or do you think that's merge blocker?

> The third problem is that I'm not even sure that we can do that.  The list of
> symbols that end up being found by gtk-doc tends to change with the options and
> platform that we build GLib with.  I fear that we may end up in a situation
> where we permanently break 'make check' on every platform other than Linux...

We could enable that check only for linux builds if it turns out it breaks everything else (it probably does tbh). Also in tp-glib that check is enabled only when building dev version (version x.y.z.1), not release tarballs (version x.y.z). I don't know if glib has a way in its build system to detect that?

The point is that a few symbols are missing in the doc, I realized that when looking for g_output_stream_write_all_async() and friends. That mistake will repeat forever unless we make the build fail if doc is missing.
Comment 6 Xavier Claessens 2014-11-28 15:53:58 UTC
btw, I already made libgobject pass the test, and I'm working on libgio atm. I hope to finish it this weekend.
Comment 7 Allison Karlitskaya (desrt) 2014-11-28 17:53:09 UTC
(In reply to comment #5)
> The point is that a few symbols are missing in the doc, I realized that when
> looking for g_output_stream_write_all_async() and friends. That mistake will
> repeat forever unless we make the build fail if doc is missing.

I completely agree that this is worth fixing.

I think your proposal to enable it only on Linux builds is a good pragmatic solution.
Comment 8 Xavier Claessens 2014-11-28 20:56:37 UTC
Reported bug #740875 with patch against gtk-doc.
Comment 9 Matthias Clasen 2014-11-29 00:39:54 UTC
My opinion: this approach does not help really help. Unless you force patch committers to run make, all this does is make it more likely that I release a tarball that is generated with make dist instead of distcheck.
Comment 10 Xavier Claessens 2014-11-29 13:45:22 UTC
Created attachment 291775 [details] [review]
Doc: glib: Fix all undocumented/unused/undeclared symbols
Comment 11 Xavier Claessens 2014-11-29 13:46:32 UTC
Created attachment 291776 [details] [review]
Doc: gobject: Fix all undocumented/unused/undeclared symbols
Comment 12 Xavier Claessens 2014-11-29 13:46:36 UTC
Created attachment 291777 [details] [review]
Doc: gio: Fix all undocumented/unused/undeclared symbols
Comment 13 Xavier Claessens 2014-11-29 13:46:40 UTC
Created attachment 291778 [details] [review]
Doc: enable gtk-doc checks on linux

This fail "make check" if any symbol is missing from the doc.
This will ensure we stay at 100% coverage.
Comment 14 Xavier Claessens 2014-11-29 13:49:06 UTC
(In reply to comment #9)
> My opinion: this approach does not help really help. Unless you force patch
> committers to run make, all this does is make it more likely that I release a
> tarball that is generated with make dist instead of distcheck.

Forcing committers to run make check looks like a sane idea.
Comment 15 Xavier Claessens 2014-11-29 14:02:33 UTC
One thing I noticed is that doc for GTypeValueTable is totally broken since the change to use markdown. The problem is you cannot have empty line on item doc otherwise gtkdoc think that starts the enum description. But you cannot use markdown list without an empty line to end it.
Comment 16 Xavier Claessens 2014-12-01 18:50:36 UTC
oh, also notice that I moved G_PARAM_READWRITE out of the enum and made it a macro, because gtkdoc didn't like having such value in the enum and that symbol never appeared in the doc. I believe this is not API/ABI break, right?
Comment 17 Emmanuele Bassi (:ebassi) 2014-12-01 18:53:40 UTC
(In reply to comment #16)
> oh, also notice that I moved G_PARAM_READWRITE out of the enum and made it a
> macro, because gtkdoc didn't like having such value in the enum and that symbol
> never appeared in the doc.

then gtk-doc needs to be fixed, because I moved the symbol in the enumeration for a reason — see bug #726037.

I'm pretty sure gtk-doc survived when I built the documentation after fixing that bug, but you may have an older version.
Comment 18 Xavier Claessens 2014-12-01 19:10:11 UTC
Please double check, but I think you broke the doc and didn't notice precisely because we don't fail "make check" when symbols are not documented. At least here G_PARAM_READWRITE is undocumented with gtkdoc master.

Note that I could also say fix introspection scanner about bug #726037.

A workaround would be to give the numerical value and tell in a comment that it's READ|WRITE, like I had to do in GTokenType.
Comment 19 Xavier Claessens 2014-12-01 19:19:19 UTC
Hm, or maybe jhbuild is playing tricks to me and I'm not using gtkdoc master? I'm not 100% sure :/
Comment 20 Xavier Claessens 2014-12-08 19:49:40 UTC
Created attachment 292324 [details] [review]
Doc: glib: Fix all undocumented/unused/undeclared symbols
Comment 21 Xavier Claessens 2014-12-08 19:49:46 UTC
Created attachment 292325 [details] [review]
Doc: gobject: Fix all undocumented/unused/undeclared symbols
Comment 22 Xavier Claessens 2014-12-08 19:49:53 UTC
Created attachment 292326 [details] [review]
Doc: gio: Fix all undocumented/unused/undeclared symbols
Comment 23 Xavier Claessens 2014-12-08 19:49:59 UTC
Created attachment 292327 [details] [review]
Doc: enable gtk-doc checks on linux

This fail "make check" if any symbol is missing from the doc.
This will ensure we stay at 100% coverage.
Comment 24 Xavier Claessens 2014-12-08 19:52:42 UTC
@Emmanuele: you were right, reverted my change for G_PARAM_READWRITE, the problem was more trivial: you forgot to remove it from the sections.txt :-)

Rebased my patches on top of master, where GNetworkConnectivity and friends got added and doc was missing (proofing my point...)
Comment 25 Allison Karlitskaya (desrt) 2014-12-08 21:58:07 UTC
Review of attachment 292324 [details] [review]:

Looks like a lot of good improvements, but please see a few outstanding issues below.

::: glib/ghash.c
@@ +1309,3 @@
  * Checks if @key is in @hash_table.
  *
+ * Returns: %TRUE of @key is in @hash_table, %FALSE otherwise.

s/of/if/

Also: please an extra line between Returns: and Since:

::: glib/gkeyfile.c
@@ +397,3 @@
+ *
+ * A key under #G_KEY_FILE_DESKTOP_GROUP, whose value is a boolean set to true
+ * if the application is dbus activatable.

we typically call it like "D-Bus" in our docs.

@@ +406,3 @@
+ *
+ * A key under #G_KEY_FILE_DESKTOP_GROUP, whose value is a string list
+ * giving the available application #GAction.

just say "application actions"

::: glib/gscanner.h
@@ +80,3 @@
   G_TOKEN_RIGHT_PAREN		= ')',
+  G_TOKEN_LEFT_CURLY		= 0x7b, /* gtk-doc doesn't like '{' */
+  G_TOKEN_RIGHT_CURLY		= 0x7d, /* gtk-doc doesn't like '}' */

No.  gtk-doc needs to be fixed.

::: glib/gstdio.c
@@ +849,1 @@
  * Since: 2.36

Extra empty line please.

::: glib/valgrind.h
@@ +73,3 @@
 #define __VALGRIND_H
 
+#ifndef __GTK_DOC_IGNORE__

I'd prefer if we could avoid adding things like this to this (imported) file.  Can we just put it on IGNORE_HFILES instead?
Comment 26 Allison Karlitskaya (desrt) 2014-12-08 22:05:57 UTC
Review of attachment 292325 [details] [review]:

::: docs/reference/gobject/gobject-sections.txt
@@ -885,3 @@
-<SUBSECTION>
-g_cclosure_marshal_VOID__VOID
-g_cclosure_marshal_VOID__BOOLEAN

Why do you feel that it's appropriate to delete these?  Is it just to avoid writing docstrings for each one?  Admittedly that sounds painful...

I often do a quick check in devhelp to see which of these are available...

::: gobject/gtype.h
@@ +415,3 @@
+  gdouble  v_double;
+  gpointer v_pointer;
+};

Why are you moving around code in a docs patch?
Comment 27 Xavier Claessens 2014-12-09 18:19:34 UTC
(In reply to comment #25)
> ::: glib/gscanner.h
> @@ +80,3 @@
>    G_TOKEN_RIGHT_PAREN        = ')',
> +  G_TOKEN_LEFT_CURLY        = 0x7b, /* gtk-doc doesn't like '{' */
> +  G_TOKEN_RIGHT_CURLY        = 0x7d, /* gtk-doc doesn't like '}' */
> 
> No.  gtk-doc needs to be fixed.

https://bugzilla.gnome.org/show_bug.cgi?id=741305
Comment 28 Xavier Claessens 2014-12-10 15:38:31 UTC
(In reply to comment #26)
> Review of attachment 292325 [details] [review]:
> 
> ::: docs/reference/gobject/gobject-sections.txt
> @@ -885,3 @@
> -<SUBSECTION>
> -g_cclosure_marshal_VOID__VOID
> -g_cclosure_marshal_VOID__BOOLEAN
> 
> Why do you feel that it's appropriate to delete these?  Is it just to avoid
> writing docstrings for each one?  Admittedly that sounds painful...
> 
> I often do a quick check in devhelp to see which of these are available...

Was too lazy to write the doc for all of them, since they should be replaced with generic marshaller anyway. But I wrote the doc now.

> ::: gobject/gtype.h
> @@ +415,3 @@
> +  gdouble  v_double;
> +  gpointer v_pointer;
> +};
> 
> Why are you moving around code in a docs patch?

For some reason gtk-doc does not find it in gvaluecollector.h, if you check current doc you'll see that all members are missing. I think that's because it should be in same module than the typedef.
Comment 29 Xavier Claessens 2014-12-10 15:39:03 UTC
Created attachment 292450 [details] [review]
fixup! Doc: glib: Fix all undocumented/unused/undeclared symbols
Comment 30 Xavier Claessens 2014-12-10 15:39:08 UTC
Created attachment 292451 [details] [review]
fixup! Doc: gobject: Fix all undocumented/unused/undeclared symbols
Comment 31 Matthias Clasen 2014-12-10 18:20:57 UTC
(In reply to comment #28)
 
> Was too lazy to write the doc for all of them, since they should be replaced
> with generic marshaller anyway. But I wrote the doc now.
> 

If that was the case, they would be deprecated in favor of the generic marshaller. But sadly, the ffi-based marshaller is slower, which is why these are still around and undeprecated.
Comment 32 Allison Karlitskaya (desrt) 2014-12-12 16:02:21 UTC
Attachment 292324 [details] pushed as 1a2a689 - Doc: glib: Fix all undocumented/unused/undeclared symbols
Comment 33 Xavier Claessens 2014-12-12 16:37:43 UTC
Created attachment 292620 [details] [review]
Doc: enable gtkdoc-check on linux

This fail "make check" if any symbol is missing from the doc.
This will ensure we stay at 100% coverage.
Comment 34 Xavier Claessens 2014-12-12 16:38:32 UTC
Review of attachment 292327 [details] [review]:

No need, gtk-doc already had his way of checking the doc.
Comment 35 Xavier Claessens 2014-12-12 18:04:27 UTC
Created attachment 292624 [details] [review]
Doc: gobject: Fix all undocumented/unused/undeclared symbols
Comment 36 Xavier Claessens 2014-12-15 18:57:24 UTC
Created attachment 292765 [details] [review]
Doc: gio: Fix all undocumented/unused/undeclared symbols
Comment 37 Xavier Claessens 2014-12-15 18:57:59 UTC
Created attachment 292766 [details] [review]
configure.ac: Add ENABLE_GTK_DOC_CHECK conditional
Comment 38 Xavier Claessens 2014-12-15 18:58:07 UTC
Created attachment 292767 [details] [review]
Doc: gobject: enable gtkdoc-check
Comment 39 Xavier Claessens 2014-12-15 18:58:12 UTC
Created attachment 292768 [details] [review]
Doc: glib: enable gtkdoc-check
Comment 40 Xavier Claessens 2014-12-15 18:58:18 UTC
Created attachment 292769 [details] [review]
Doc: gio: enable gtkdoc-check
Comment 41 Xavier Claessens 2014-12-19 19:54:23 UTC
Not re-attaching patches again, did a few small changes, branch is there:
http://cgit.collabora.com/git/user/xclaesse/glib.git/log/?h=doc
Comment 42 Allison Karlitskaya (desrt) 2015-02-05 15:08:18 UTC
Okay -- I've applied a lot of these patches with some modifications.

I didn't apply any of the ones for enabling the checks by default.  Despite what I said earlier, I've slowly become vaguely unimpressed by the Linux-only approach.  I think I'd really prefer to just have file containing a list of symbols that we know to be problematic.

In terms of the "big patches" all that's left is the biggest of all (the GIO one) plus one small part of the GObject patch that I didn't apply: what's the deal with the moving of the declaration of GTypeCValue?  That seems suspicious and is not explained at all.
Comment 43 Allison Karlitskaya (desrt) 2015-02-05 15:23:26 UTC
And now most of the GIO changes are pushed as well.

I didn't include a couple of changes.  See below.

Adding @g_iface everywhere seems like we're working around a bug of some kind.  We should probably fix that.

Likewise the shuffling of < private > and < public > in various vtables.

Why did you ignore gnetworking.h?
Comment 44 Xavier Claessens 2015-02-05 16:19:54 UTC
(In reply to comment #42)
> Okay -- I've applied a lot of these patches with some modifications.

Thanks :D

> I didn't apply any of the ones for enabling the checks by default.  Despite
> what I said earlier, I've slowly become vaguely unimpressed by the Linux-only
> approach.  I think I'd really prefer to just have file containing a list of
> symbols that we know to be problematic.

I would be happy to enable on other platforms as well, it's just that can't test if it pass. But people doing windows builds should work on making it pass there as well so we can enable the check for them. IMO the linux-only approach is temporary, and I personally don't care fixing doc for other platforms than linux.

I was tempted by a blacklist file as well, but in the end it turns out that there are really few problematic symbols, and those case should be fixed in gtk-doc itself anyway. I prefer working on having no problematic symbols than working on a blacklist system.

> In terms of the "big patches" all that's left is the biggest of all (the GIO
> one) plus one small part of the GObject patch that I didn't apply: what's the
> deal with the moving of the declaration of GTypeCValue?  That seems suspicious
> and is not explained at all.

For some obscure reason gtk-doc is unhappy to have that typedef away from its doc block. I agree that case should be investigated in gtkdoc itself.
Comment 45 Xavier Claessens 2015-02-05 16:26:25 UTC
(In reply to comment #43)
> And now most of the GIO changes are pushed as well.

\o/

> Adding @g_iface everywhere seems like we're working around a bug of some kind. 
> We should probably fix that.
> 
> Likewise the shuffling of < private > and < public > in various vtables.

Would be cool to special case them in gtk-doc indeed.

> Why did you ignore gnetworking.h?

Because it #define symbols that are probably considered private (no idea what all that does tbh) so that was the easy way out. On 2nd though I could add them in a <SUBSECTION Private> in sections.txt.
Comment 46 Xavier Claessens 2015-02-05 17:25:00 UTC
(In reply to comment #45)
> > Adding @g_iface everywhere seems like we're working around a bug of some kind. 
> > We should probably fix that.
> > 
> > Likewise the shuffling of < private > and < public > in various vtables.
> 
> Would be cool to special case them in gtk-doc indeed.

Adding this to gtk-doc fix it:
    # Remove g_iface, parent_instance and parent_class if they are first member
    $declaration =~ s/(\{)\s*(\w)+\s+(g_iface|parent_instance|parent_class)\s*;/$1/g;

But then we have to remove all the @parent_instance args from our doc. Does that sounds good to you?
Comment 47 Xavier Claessens 2015-02-05 17:29:04 UTC
reported bug #744061
Comment 48 Xavier Claessens 2015-02-05 19:45:57 UTC
Adopting the wip/$username/$branch convention from now on, I've pushed my work in git.gnome.org instead of collabora server.

 - wip/xclaesse/doc-part1: Rebased on top of what you already merged in master. It requires gtk-doc's wip/xclaesse/fixes-for-glib. The only pending issue is GTypeCValue.

 - wip/xclaesse/doc-part2: Enable gtkdoc-check on linux.
Comment 49 GNOME Infrastructure Team 2018-05-24 17:17:12 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/963.