Subject: Re: [bug#61686] [PATCH] gnu: Add dropwatch.

From: Bruno Victal <[email-masked]>
Date: Sat, 25 Feb 2023 19:21:58 +0000

Hi Nicolas,

On 2023-02-24 10:44, Nicolas Goaziou wrote:
>> Note: the kernel option NET_DROP_MONITOR must be set for this package
>> to work.
> 
> Do you think it is worth mentioning in the description? AFAICT, Guix
> does not set this option by default.

With e701f67246680c81dd111cf7136b936bf7d8718b, this is now set by default.

>> +(define-public dropwatch
>> +  (package
>> +    (name "dropwatch")
>> +    (version "1.5.4")
>> +    (source (origin
>> +              (method git-fetch)
>> +              (uri (git-reference
>> +                    (url "https://github.com/nhorman/dropwatch.git")
> 
> You can drop the ".git" suffix here.

Is there a reason for dropping the suffix for GitHub? AFAIK it "works" but it's not
the same url that they give out for cloning.

>> +                    (commit (string-append "v" version))))
> 
> You also need to add
> 
>   (file-name (git-file-name name version))
> 
> for proper naming in the store.

Is this always required? I assumed the default if unspecified was "good enough" and
that snippet was used for repositories with problematic names.

>> +    (home-page "https://github.com/nhorman/dropwatch")
>> +    (synopsis "Kernel dropped packet monitor")
>> +    (description
>> +     "dropwatch is an interactive utility for monitoring and
> 
> Nicpick: dropwatch —> Dropwatch

Noted.

> 
>> +recording packets that are dropped by the kernel. It provides the commands
> 
> You need separate sentences with two spaces, per Texinfo syntax.

This is actually not strictly necessary for strings embedded in scm files. The texinfo extractor is smart enough to automatically space the strings for us here.
You can verify this behavior with 'guix search dropwatch' and see that it automatically inserts two spaces correctly.

Cheers,
Bruno