Subject: Re: [bug#68946] [PATCH v2] guix: Add logging module.

From: Simon Tournier <[email-masked]>
Date: Mon, 19 Feb 2024 16:57:28 +0100

Hi Maxim,

On ven., 16 févr. 2024 at 14:03, Maxim Cournoyer <[email-masked]> wrote:

> Most of that code also exists in guile-hall, was released in 2023, hence
> the copyright year start [0].

I think it would be best something like:

--8<---------------cut here---------------start------------->8---
;;; Copyright © 2024 Maxim Cournoyer <[email-masked]>
;;;
;;; The procedures ’define-log-level', ’setup-logging’ and
;;; ’shutdown-logging’ are taken from the 'hall/logging.scm' file of
;;; Guile-lib (guile-hall). That file has the following copyright
;;; notice:
;;;
;;; Copyright © 2023 Maxim Cournoyer <[email-masked]>
;;;
;;; This file is part of Guix.
;;;
;;; GNU Guix is distributed in the hope that it will be useful, but
…
--8<---------------cut here---------------end--------------->8---

> [0]  https://gitlab.com/a-sassmannshausen/guile-hall/-/blob/master/hall/logging.scm
>
>>> +(define-syntax define-log-level
>>> +  ;; This macro defines a log-level enum type bound to ENUM-NAME for the
>>> +  ;; provided levels.  The levels should be specified in increasing order of
>>> +  ;; severity.  It also defines 'log-LEVEL' syntax to more conveniently log at
>>> +  ;; LEVEL, with location information.
>>
>> Why not also a docstring?
>
> Only procedures can have docstrings, unfortunately.

Ah?  For instance ’define-with-syntax-properties’ or ’define-diagnostic’
or ’leave’ from (guix diagnostics); other instance in (gnu build
accounts) or (gnu build secret-service) etc.

Do I miss something?

>> This make “--log-level debug” valid, right?
>>
>> I think the convention is --long-option[=PARAMETER] and that
>> --long-option PARAMTER is unconventional.  Although I do not find the
>> reference.
>
> It's not as much a convention as a limitation of the SRFI 37 option
> parser.  GNU getopt, which SRFI 37 aims to emulate,  doesn't have such a
> limitation, for example.  We should improve SRFI 37 to lift such
> limitation, in my opinion.

I think that’s a bad choice.  For two reasons,

 1. Let’s keep an uniform across subcommands

 2. It potentially opens bug as #50472; no space avoids ambiguous
    implementations as explained. :-)

    https://issues.guix.gnu.org/issue/50472

Cheers,
simon]