Bug 63573

Created attachment 78031 [details] [review]
0001-Add-annotation-glossary.patch

Created attachment 78032 [details] [review]
0002-Leave-out-backend-from-gtk-doc-generation.patch

Created attachment 78033 [details] [review]
0003-Fix-most-undocumented-symbol-warnings.patch

Created attachment 78034 [details] [review]
0004-Move-polkit_temporary_authorization_new-to-private-h.patch

Created attachment 78035 [details] [review]
0005-Include-documentation-of-polkit_action_description_g.patch

Created attachment 78036 [details] [review]
0006-Document-deprecated-functions.patch

I hope this won't break introspection.

Created attachment 78037 [details] [review]
0007-Fold-enum-documentation-into-relevant-classes.patch

Created attachment 78038 [details] [review]
0008-Fix-an-obvious-docstring-typo.patch

Created attachment 78039 [details] [review]
0009-Add-annotations-for-element-types-of-returned-lists.patch

Not 100% sure about this - only tested that g-ir-scanner doesn't warn any more, didn't try to use the bindings.

(I can push the patches to git now, so it's not necessary to manually extract the patches from bugzilla.  A review would of course be appreciated.)

Comment on attachment 78034 [details] [review]
0004-Move-polkit_temporary_authorization_new-to-private-h.patch

Review of attachment 78034 [details] [review]:
-----------------------------------------------------------------

Unfortunately though polkit uses:

libpolkit_gobject_1_la_LDFLAGS = -export-symbols-regex '(^polkit_.*)'

In my projects I tend to always use "_" before unexported symbols; another alternative is G_GNUC_INTERNAL, if these things should really not be public.

For the introspection one, you can use (skip) to tell the scanner not to bind it.

Regardless the patches look great to me, thanks!

(In reply to comment #10)
> Comment on attachment 78034 [details] [review] [review]
> 0004-Move-polkit_temporary_authorization_new-to-private-h.patch
> 
> Review of attachment 78034 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> Unfortunately though polkit uses:
> 
> libpolkit_gobject_1_la_LDFLAGS = -export-symbols-regex '(^polkit_.*)'
> 
> In my projects I tend to always use "_" before unexported symbols; another
> alternative is G_GNUC_INTERNAL, if these things should really not be public.

Even worse, polkitprivate.h is installed along with other public headers ( /usr/include/polkit-1/polkit/polkitprivate.h ).

I'm not currently out to clean up every single minor detail (the gtk-doc series is primarily motivated by better efficiency with emacs' M-x compile), and I'm especially not enthusiastic about risking breaking ABI for no user benefit.  Given that some of the functions in polkitprivate.h are at somewhat plausible to be useful for applications, I'll just add a FIXME/warning to polkitprivate.h , to mark it for possible cleanup in some far future in which an ABI break might have become unavoidable.

(OTOH polkit_temporary_authorization_new does disappear from the introspection by moving it to the *private.h file.  In this case I think the risk is low enough; again I have checked with searchco.de.)


(In reply to comment #11)
> For the introspection one, you can use (skip) to tell the scanner not to
> bind it.

I have checked the generated .gir before and after the patch - they only differ by added <doc>, so it should be safe.

Thanks for the review, will push after bug 63692 is resolved.