Subject: Re: [bug#32346] [PATCH 1/6] services: tor: Add a system test.
From: Chris Marusich <[email-masked]>
To: [email-masked] (Ludovic Courtès)
Cc: [email-masked]
Date: Tue, 28 Aug 2018 00:46:05 -0700
List-Id: <debbugs-submit.debbugs.gnu.org>

Hi Ludo,

I've incorporated your feedback and committed this patch series as
3bcb305b98e02f6c9d98e7325813fc00f18f0e6c.  Details follow.

[email-masked] (Ludovic Courtès) writes:

> Chris Marusich <[email-masked]> skribis:
>
>
> [...]
>
>> +      ;; Allow TOR to write its PID file.
>
> Nitpick: I think the maintainers no longer consider the name an acronym
> and write it as “Tor”, not “TOR”.

This is good to know.  In each patch that uses the incorrect name "TOR",
I've changed it to use the correct name "Tor".

>> +(define* (wait-for-unix-socket path marionette
>
> Super nitpick: s/path/file/ or s/path/file-name/, as per GNU
> convention (where “path” means “search path”.)

I've updated this to follow the convention.

>> +@deftp {Data Type} tor-configuration
>> +@table @asis
>> +@item @code{tor} (default: @code{tor})
>> +The package that provides the TOR daemon.  This package is expected
>> to provide
>> +the daemon at @file{bin/tor} relative to its output directory.  The default
>> +package is the @uref{https://www.torproject.org, TOR Project's}
>> +implementation.
>> +@item @code{config-file} (default: @code{(plain-file "empty" "")})
>
> You could skip a line between between each @item for clarity.

Good idea.  I've done this, too.

> Thank you for this nice patch series!

Thank you for the thoughtful review!

I appreciate your attention to detail.  If you hadn't told me, I
wouldn't have known about some of these things, such as the GNU
convention to use "file" or "file-name" instead of "path" when speaking
about UNIX domain sockets.  I'll remember these things going forward.

And with that, I will now close this patch!

-- 
Chris