Subject: Re: bug#60788: [PATCH] services: Add vnstat-service-type.

From: Maxim Cournoyer <[email-masked]>
Date: Thu, 09 Feb 2023 08:31:48 -0500

Hi Bruno,

Bruno Victal <[email-masked]> 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