Hello again,

I said I could do it in a week because I've just spent the weekend learning enough about wrangling Neovim to put some macros together to speed up the checking and deletion processes massively. I can now jump between symbols, bring up their header declarations side-by-side, if they exist, and delete their redundant doc comments, with only a few keypresses.

I've performed this process on a couple of CADET source files and attached the results as patches. Has the process produced the right results?

All I need now is a way of rapidly navigating between source files in all of the different directories.

Best wishes,
        Willow

On 10/06/2022 14:48, Christian Grothoff wrote:
On 6/10/22 3:42 PM, Willow Liquorice wrote:
Oh, I didn't know we already had a standard. I'll remember that and put
it in the updated Style Guide.

:-)

It's a good thing the team has a self-appointed documentation busybody
now, hm?

A very good thing. ;-)

Provided that the more recent docs are the ones in the headers,
I'll have the source docs deduplicated by the end of next week.

That's largely a safe assumption, just make sure to NEVER remove
comments from 'static' functions. If the function has a very long
detailed comment, you may need to check that the header does indeed have
that comment and is not only an abbreviated version, but 95% of the time
they will be identical.

Anyway, these are the modules that I'm unsure about how to categorise:
     * "Load library"
     * "CURL integration library"
     * "NAT testing library"
     * "MySQL library"
     * "Friends library"
     * "Credential service"
     * "Resolver service"
     * "Network type categorisation"
     * "SOCKS proxy"

What do they do, and do any of these pertain to specific GNUnet subsystems?

MySQL (together with Postgres and Sqlite) are general helper functions
to talk to the respective database; they are often used with multiple
subsystems.

Load -- I think that's largely historic code to detect the current load
on the local system to allow FS (and theoretically other subsystems) to
adapt their behavior to system load.

CURL: generic http client logic

Resolver: generic DNS client logic

NAT testing/network type catgorization: helper subsystems for transport

Friends: parsing for 'topology' (gossip overlay / bootstrapping)

SOCKS: proxy logic for GNU Name System

Credential: GNS/Re:claimID.


On 10/06/2022 07:04, Christian Grothoff wrote:
On 6/9/22 23:52, Willow Liquorice wrote:
I've categorised the code as much as I can, though I'm not too sure
about the function of some modules and I've only categorised within
src/include, so there's still a bit that's unsorted. It's mostly
services that are undocumented elsewhere, and libraries of uncertain
function that I couldn't establish as part of libgnunetutil.

I've also knocked a rudimentary script together, to process the literal
torrent of warnings Doxygen emits when I build the source docs. I've
used regexes to categorise them, and here are the results:
      * 5655 occurrences of parameters with multiple docs across 175
source files.
      * 3372 documented parameters which do not exist in their
corresponding function definitions, across 276 source files.
      * 1660 functions with undocumented parameters, across 282 source
files.
      * 760 explicit links to other parts of the documentation cannot be
resolved.
      * 2-300 errors across a laundry list of other categories.

This is a rather sorry state of affairs.

Those redundant parameters reflect a common phenomenon in the
repository, where a function is documented in multiple locations. I'm
not sure about the reasons for doing this, but in my opinion it
unnecessarily increases the maintenance load as documentation has to be
updated in multiple locations.

Right, which is why for a few years we've been eliminating the
duplicates, always only keeping the version in the header if a function
is non-static. Alas, as you said above, there is a very large number of
these, and so progress is slow but steady.

I think a code style decision needs to be made, about where to locate
docs, and the redundant documentation removed.

Headers, otherwise (static prototypes) on first occurrence.

I've also found, in my experiments with the new Sphinx docs, that the
redundantly-documented parameters are counted twice by Breathe, which
could motivate stripping the redundant docs if we decide to go ahead
with integrating source code docs.

What do people think?

We've been doing this, but basically only whenever we edit a source file
already, due to, eh, "capacity" issues ;-).

Cheers!

Christian

On 06/06/2022 09:27, Martin Schanzenbach wrote:
Hi,

that sounds like a good approach to me :)

Thanks
Martin

On Sun, 2022-06-05 at 23:59 +0100, Willow Liquorice wrote:
Hello everyone,

I was going to mention this at the Dev Meeting today, but Mumble
wasn't
playing very nicely with my OS' audio system.

Another thing I've decided to work on is making the Doxygen site more
user-friendly, as I found out that it's possible to nest groups.

I've used this capability to make the Modules page easier to navigate
by
adding an layer or two of domain-specific grouping on top of the
current
modules (putting all the libgnunetutil headers into their own group,
the
network backbone going into one group, etc. etc.).

Does this sound like a good idea, and what would your recommendations
be
for how to organise the modules if so?

Best wishes,
           Willow






From 42763e19c771aa332a2a2307ad05f45a3da87dd0 Mon Sep 17 00:00:00 2001
From: Willow Liquorice <wil...@howhill.com>
Date: Sun, 12 Jun 2022 20:30:40 +0100
Subject: [PATCH] Doxygen: First redundant comment stripping results

---
 src/cadet/gnunet-service-cadet_dht.c | 26 --------------------------
 1 file changed, 26 deletions(-)

diff --git a/src/cadet/gnunet-service-cadet_dht.c b/src/cadet/gnunet-service-cadet_dht.c
index d44a50f506..ff057efdc1 100644
--- a/src/cadet/gnunet-service-cadet_dht.c
+++ b/src/cadet/gnunet-service-cadet_dht.c
@@ -55,9 +55,6 @@
 #define LOG(level, ...) GNUNET_log_from (level, "cadet-dht", __VA_ARGS__)
 
 
-/**
- * Handle for DHT searches.
- */
 struct GCD_search_handle
 {
   /**
@@ -212,10 +209,6 @@ announce_id (void *cls)
 }
 
 
-/**
- * Function called by the HELLO subsystem whenever OUR hello
- * changes. Re-triggers the DHT PUT immediately.
- */
 void
 GCD_hello_update ()
 {
@@ -229,11 +222,6 @@ GCD_hello_update ()
 }
 
 
-/**
- * Initialize the DHT subsystem.
- *
- * @param c Configuration.
- */
 void
 GCD_init (const struct GNUNET_CONFIGURATION_Handle *c)
 {
@@ -274,9 +262,6 @@ GCD_init (const struct GNUNET_CONFIGURATION_Handle *c)
 }
 
 
-/**
- * Shut down the DHT subsystem.
- */
 void
 GCD_shutdown (void)
 {
@@ -293,12 +278,6 @@ GCD_shutdown (void)
 }
 
 
-/**
- * Search DHT for paths to @a peeR_id
- *
- * @param peer_id peer to search for
- * @return handle to abort search
- */
 struct GCD_search_handle *
 GCD_search (const struct GNUNET_PeerIdentity *peer_id)
 {
@@ -335,11 +314,6 @@ GCD_search (const struct GNUNET_PeerIdentity *peer_id)
 }
 
 
-/**
- * Stop DHT search started with #GCD_search().
- *
- * @param h handle to search to stop
- */
 void
 GCD_search_stop (struct GCD_search_handle *h)
 {
-- 
2.25.1

From 27182aaf8b6308f4208b58acf93cc5e09d51687a Mon Sep 17 00:00:00 2001
From: Willow Liquorice <wil...@howhill.com>
Date: Sun, 12 Jun 2022 20:53:35 +0100
Subject: [PATCH] Doxygen: second redundant comment stripping

---
 src/cadet/gnunet-service-cadet_core.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/src/cadet/gnunet-service-cadet_core.c b/src/cadet/gnunet-service-cadet_core.c
index 95a5d3f639..bff7632da0 100644
--- a/src/cadet/gnunet-service-cadet_core.c
+++ b/src/cadet/gnunet-service-cadet_core.c
@@ -139,14 +139,6 @@ struct RouteDirection
 };
 
 
-/**
- * Description of a segment of a `struct CadetConnection` at the
- * intermediate peers.  Routes are basically entries in a peer's
- * routing table for forwarding traffic.  At both endpoints, the
- * routes are terminated by a `struct CadetConnection`, which knows
- * the complete `struct CadetPath` that is formed by the individual
- * routes.
- */
 struct CadetRoute
 {
   /**
@@ -1321,9 +1313,6 @@ GCO_init (const struct GNUNET_CONFIGURATION_Handle *c)
 }
 
 
-/**
- * Shut down the CORE subsystem.
- */
 void
 GCO_shutdown ()
 {
-- 
2.25.1

Reply via email to