GNU bug report logs - #60788
[PATCH] services: Add vnstat-service-type.

Previous Next

Full log

View this message in rfc822 format

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Bruno Victal <mirai <at> makinata.eu>
Cc: 60788 <at> debbugs.gnu.org
Subject: [bug#60788] [PATCH] services: Add vnstat-service-type.
Date: Thu, 09 Feb 2023 08:31:48 -0500

Hi Bruno,

Bruno Victal <mirai <at> makinata.eu> writes:

> Hi Maxim,
>
> On 2023-02-09 03:34, Maxim Cournoyer wrote:
>> Two remaining comments from me:
>>
>> 1. I'd normalize all the configuration field names to lowercase, so
>> max-BW and use-UTC? would become max-bw and use-utc?, for example.
>
> No, this is intentional so that uglify+case-conversion yields correct configuration directives.

OK; the case conversion could be special-cased (max-bw -> max-BW,
use-utc? -> utc-UTC?) in the uglify+case-conversion logic.  But it seems
that vnstat doesn't care about case when parsing its options; it uses
strcasecmp, which ignores the case of the characters per 'man 3
strcasecmp' [0].

[0]  https://github.com/vergoh/vnstat/blob/master/src/cfg.c#L149

>> 2. I'd add a simple example vnstat usage example to the manual.
>
> The service is "ready to be used" with the default values, so I didn't think
> it'd add much value adding an example here. Do you think we should add one?

I think so; with so many switches it can appear intimidating; but a
minimal working example would show to users that they can start simple.

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.