[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[bug#60788] [PATCH] services: Add vnstat-service-type.
From: |
Maxim Cournoyer |
Subject: |
[bug#60788] [PATCH] services: Add vnstat-service-type. |
Date: |
Thu, 09 Feb 2023 08:31:48 -0500 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux) |
Hi Bruno,
Bruno Victal <mirai@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