Bug 63573 - [Patch] Mostly clean up gtk-doc
Summary: [Patch] Mostly clean up gtk-doc
Status: RESOLVED FIXED
Alias: None
Product: PolicyKit
Classification: Unclassified
Component: daemon (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Miloslav Trmac
QA Contact: David Zeuthen (not reading bugmail)
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-15 21:21 UTC by Miloslav Trmac
Modified: 2013-04-19 19:28 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
0001-Add-annotation-glossary.patch (878 bytes, patch)
2013-04-15 21:23 UTC, Miloslav Trmac
Details | Splinter Review
0002-Leave-out-backend-from-gtk-doc-generation.patch (1.00 KB, patch)
2013-04-15 21:24 UTC, Miloslav Trmac
Details | Splinter Review
0003-Fix-most-undocumented-symbol-warnings.patch (1.87 KB, patch)
2013-04-15 21:24 UTC, Miloslav Trmac
Details | Splinter Review
0004-Move-polkit_temporary_authorization_new-to-private-h.patch (3.25 KB, patch)
2013-04-15 21:25 UTC, Miloslav Trmac
Details | Splinter Review
0005-Include-documentation-of-polkit_action_description_g.patch (1.07 KB, patch)
2013-04-15 21:25 UTC, Miloslav Trmac
Details | Splinter Review
0006-Document-deprecated-functions.patch (2.83 KB, patch)
2013-04-15 21:26 UTC, Miloslav Trmac
Details | Splinter Review
0007-Fold-enum-documentation-into-relevant-classes.patch (2.67 KB, patch)
2013-04-15 21:27 UTC, Miloslav Trmac
Details | Splinter Review
0008-Fix-an-obvious-docstring-typo.patch (785 bytes, patch)
2013-04-15 21:27 UTC, Miloslav Trmac
Details | Splinter Review
0009-Add-annotations-for-element-types-of-returned-lists.patch (4.42 KB, patch)
2013-04-15 21:29 UTC, Miloslav Trmac
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Miloslav Trmac 2013-04-15 21:21:57 UTC
... at least enough so that emacs' "M-x compile" doesn't report any warnings.
Comment 1 Miloslav Trmac 2013-04-15 21:23:35 UTC
Created attachment 78031 [details] [review]
0001-Add-annotation-glossary.patch
Comment 2 Miloslav Trmac 2013-04-15 21:24:10 UTC
Created attachment 78032 [details] [review]
0002-Leave-out-backend-from-gtk-doc-generation.patch
Comment 3 Miloslav Trmac 2013-04-15 21:24:45 UTC
Created attachment 78033 [details] [review]
0003-Fix-most-undocumented-symbol-warnings.patch
Comment 4 Miloslav Trmac 2013-04-15 21:25:14 UTC
Created attachment 78034 [details] [review]
0004-Move-polkit_temporary_authorization_new-to-private-h.patch
Comment 5 Miloslav Trmac 2013-04-15 21:25:36 UTC
Created attachment 78035 [details] [review]
0005-Include-documentation-of-polkit_action_description_g.patch
Comment 6 Miloslav Trmac 2013-04-15 21:26:33 UTC
Created attachment 78036 [details] [review]
0006-Document-deprecated-functions.patch

I hope this won't break introspection.
Comment 7 Miloslav Trmac 2013-04-15 21:27:07 UTC
Created attachment 78037 [details] [review]
0007-Fold-enum-documentation-into-relevant-classes.patch
Comment 8 Miloslav Trmac 2013-04-15 21:27:30 UTC
Created attachment 78038 [details] [review]
0008-Fix-an-obvious-docstring-typo.patch
Comment 9 Miloslav Trmac 2013-04-15 21:29:42 UTC
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 10 Colin Walters 2013-04-16 08:11:46 UTC
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.
Comment 11 Colin Walters 2013-04-16 08:14:48 UTC
For the introspection one, you can use (skip) to tell the scanner not to bind it.

Regardless the patches look great to me, thanks!
Comment 12 Miloslav Trmac 2013-04-18 18:01:36 UTC
(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.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.